Re: 2.5.20 RAID5 compile error

[Jens Axboe]

On Wed, Jun 05 2002, Neil Brown wrote:
> On Tuesday June 4, [e-mail] wrote:
> > On Tue, Jun 04 2002, Jens Axboe wrote:
> > > On Tue, Jun 04 2002, Jens Axboe wrote:
> > > > plug? Wrt umem, it seems you could keep 'card' in the queuedata. Same
> > > > for raid5 and conf.
> > > 
> > > Ok by actually looking at it, both card and conf are the queues
> > > themselves. So I'd say your approach is indeed a bit overkill. I'll send
> > > a patch in half an hour for you to review.
> > 
> > Alright here's the block part of it, you'll need to merge your umem and
> > raid5 changes in with that. I'm just interested in knowing if this is
> > all you want/need. It's actually just a two line changes from the last
> > version posted -- set q->unplug_fn in blk_init_queue to our default
> > (you'll override that for umem and raid5, or rather you'll set it
> > yourself after blk_queue_make_request()), and then call that instead of
> > __generic_unplug_device directly from blk_run_queues().
> > 
> I can work with that...
> 
> Below is a patch that fixes a couple of problems with umem and add
> support for md/raid5.  I haven't tested it yet, but it looks right. It
> is on top of your patch.
> 
> It irks me, though, that I need to embed a whole request_queue_t when
> I don't use the vast majority of it.

Yeah this annoys me too...

> What I would like is to have a 
> 
> struct blk_dev {
>        	make_request_fn		*make_request_fn;
> 	unplug_fn		*unplug_fn;
> 	struct list_head	plug_list;
> 	unsigned long		queue_flags;
> 	spinlock_t		*queue_lock;
> }
> 
> which I can embed in mddev_s and umem card, and you can embed in
> struct request_queue.

To some extent, I agree with you. However, I'm not sure it's worth it
abstracting this bit out. The size of the request queue right now in
2.5.20 (with plug change) is 196 bytes on i386 SMP. If you really feel
it's worth saving these bytes, then I'd much rather go in a different
direction: only keep the "main" elements of request_queue_t, and have
blk_init_queue() alloc the "remainder"

> In ll_rw_blk.c we change blk_plug_device to avoid the check for
> the queue being empty as this may not be well define for
> umem/raid5.  Instead, blk_plug_device is not called when
> the queue is not empty (which is mostly wasn' anyway).

I left it that way on purpose, and yes I did see you changed that in the
previous patch as well. I think it's a bit of a mess to have both
blk_plug_device and blk_plug_queue. Without looking at it, I have no
idea which does what?! blk_queue_empty() will always be true for
make_request_fn type drivers, so no change is necessary there.

-- 
Jens Axboe