On 2011-04-11 14:05, NeilBrown wrote:

On Mon, 11 Apr 2011 13:37:20 +0200 Jens Axboe [off-list ref] wrote:

quoted

On 2011-04-11 13:26, NeilBrown wrote:

quoted

On Mon, 11 Apr 2011 13:04:26 +0200 Jens Axboe [off-list ref] wrote:

quoted

quoted

I'm sure one of us is missing something (probably both) but I'm not
sure what.

The callback is central.

It is simply to use plugging in md.
Just like blk-core, md will notice that a blk_plug is active and will put
requests aside.  I then need something to call in to md when blk_finish_plug

But this is done in __make_request(), so md devices should not be
affected at all. This is the part of your explanation that I do not
connect with the code.

If md itself is putting things on the plug list, why is it doing that?

Yes.  Exactly.  md itself want to put things aside on some list.
e.g. in RAID1 when using a write-intent bitmap I want to gather as many write
requests as possible so I can update the bits for all of them at once.
So when a plug is in effect I just queue the bios somewhere and record the
bits that need to be set.
Then when the unplug happens I write out the bitmap updates in a single write
and when that completes, I write out the data (to all devices).

Also in RAID5 it is good if I can wait for lots of write request to arrive
before committing any of them to increase the possibility of getting a
full-stripe write.

Previously I used ->unplug_fn to release the queued requests.  Now that has
gone I need a different way to register a callback when an unplug happens.

Ah, so this is what I was hinting at. But why use the task->plug for
that? Seems a bit counter intuitive. Why can't you just store these
internally?

quoted

quoted

quoted

is called so that put-aside requests can be released.
As md can be built as a module, that call must be a call-back of some sort.
blk-core doesn't need to register blk_plug_flush because that is never in a
module, so it can be called directly.  But the md equivalent could be in a
module, so I need to be able to register a call back.

Does that help? 

Not really. Is the problem that _you_ would like to stash things aside,
not the fact that __make_request() puts things on a task plug list?

Yes, exactly.  I (in md) want to stash things aside.

(I don't actually put the stashed things on the blk_plug, though it might
make sense to do that later in some cases - I'm not sure.  Currently I stash
things in my own internal lists and just need a call back to say "ok, flush
those lists now").

So we are making some progress... The thing I then don't understand is
why you want to make it associated with the plug? Seems you don't have
any scheduling restrictions, and in which case just storing them in md
seems like a much better option.

Yes.  But I need to know when to release the requests that I have stored.
I need to know when ->write_pages or ->read_pages or whatever has finished
submitting a pile of pages so that I can start processing the request that I
have put aside.  So I need a callback from blk_finish_plug.

OK fair enough, I'll add your callback patch.

-- 
Jens Axboe