Re: [PATCH 05/10] block: remove per-queue plugging

From: NeilBrown
Date: Sun Apr 17 2011 - 18:19:59 EST


On Mon, 11 Apr 2011 14:11:58 +0200 Jens Axboe <jaxboe@xxxxxxxxxxxx> wrote:

> > 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.
>

But you didn't did you? You added a completely different patch which is
completely pointless.
If you don't like my patch I would really prefer you said so rather than
silently replace it with something completely different (and broken).

I'll try to explain again.

md does not use __make_request. At all.
md does not use 'struct request'. At all.

The 'list' in 'struct blk_plug' is a list of 'struct request'.

Therefore md cannot put anything useful on the list in 'struct blk_plug'.

So when blk_flush_plug_list calls queue_unplugged() on a queue that belonged
to a request found on the blk_plug list, that queue cannot possibly ever be
for an 'md' device (because no 'struct request' ever belongs to an md device,
because md doesn't not use 'struct request').

So your patch (commit f75664570d8b) doesn't help MD at all.

For md, I need to attach something to blk_plug which somehow identifies an md
device, so that blk_finish_plug can get to that device and let it unplug.
The most sensible thing to have is a completely generic callback. That way
different block devices (which choose not to use __make_request) can attach
different sorts of things to blk_plug.

So can we please have my original patch applied? (Revised version using
list_splice_init included below).

Or if not, a clear explanation of why not?

Thanks,
NeilBrown