[7940] in linux-scsi channel archive

home help back first fref pref prev next nref lref last post

Re: request merge changes?

daemon@ATHENA.MIT.EDU (Jens Axboe)
Tue Jan 25 21:57:10 2000

Date:   Tue, 25 Jan 2000 17:49:43 +0100
From: Jens Axboe <axboe@suse.de>
To: Eric Youngdale <eric@andante.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, linux-scsi@vger.rutgers.edu
Message-ID: <20000125174943.C2016@suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
In-Reply-To: <001101bf6750$7f7cc070$2017a8c0@eric.home>; from eric@andante.org on Tue, Jan 25, 2000 at 11:23:21AM -0500

On Tue, Jan 25 2000, Eric Youngdale wrote:
>     Yes, I probably need to document this some.
> 
>     The basic idea is that a request_queue_t can contain some function
> pointers that are used to assist with the merging process.  There are
> essentially two functions - q->merge_fn() is used if ll_rw_blk wants to know
> if it can merge a new block into an existing request, and
> q->merge_requests_fn() is used to test to see if combining two separate
> requests would result in a single request that is too large or not.
> 
>     Both of these functions should return 1 if it is acceptable for
> ll_rw_blk to perform the merge, and should return 0 if it is not acceptable.
> As things stand, the queue merge functions are also responsible for
> maintaining the req->nr_segments field in the event that the number changes
> as a result of the merge.  ll_rw_blk also touches it some - when it creates
> a brand new request, it initializes the field to 1.

I don't think there's much need for documentation, the code is easily
readable. What I found odd is what you also note below, that there
are actually two ways to merge the requests with the new queueing code --
attempt_merge through make_request at the ll_rw_block level and the
hooks what are provided through request_queue_t (that noone currently
uses).

>     Another enhancement that I can think of would be to update
> blk_init_queue() to initialize merge_fn and merge_requests_fn to point to
> default handlers in ll_rw_blk.c.  In this way, the function pointers should
> never be NULL, and the code paths would be cleaner.  Such a change would
> also make it easier to retrofit DAC960 for that matter.

That was the path I was going to take, and IMHO is the cleanest.
Simply let merge_fn() be a ll_rw_block default function if drivers
don't want to control it themselves. Cleanup the == NULL checks, etc.

I'll do that tonight on my test box.

-- 
*  Jens Axboe <axboe@suse.de>
*  Linux CD-ROM Maintainer
*  http://www.kernel.dk

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.rutgers.edu

home help back first fref pref prev next nref lref last post