Message-ID: <Pine.LNX.4.64.0903241010490.29968@hs20-bc2-1.build.redhat.com>
Date:	Tue, 24 Mar 2009 10:26:43 -0400 (EDT)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Jens Axboe <jens.axboe@...cle.com>
cc:	device-mapper development <dm-devel@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@...e.de>,
	"MASON, CHRISTOPHER" <CHRIS.MASON@...cle.com>
Subject: Re: [dm-devel] Barriers still not passing on simple dm devices...



On Tue, 24 Mar 2009, Jens Axboe wrote:

> On Tue, Mar 24 2009, Mikulas Patocka wrote:
> > 
> > 
> > On Mon, 23 Mar 2009, Eric Sandeen wrote:
> > 
> > > I've noticed that on 2.6.29-rcX, with Andi's patch
> > > (ab4c1424882be9cd70b89abf2b484add355712fa, dm: support barriers on
> > > simple devices) barriers are still getting rejected on these simple devices.
> > > 
> > > The problem is in __generic_make_request():
> > > 
> > >                 if (bio_barrier(bio) && bio_has_data(bio) &&
> > >                     (q->next_ordered == QUEUE_ORDERED_NONE)) {
> > >                         err = -EOPNOTSUPP;
> > >                         goto end_io;
> > >                 }
> > > 
> > > and dm isn't flagging its queue as supporting ordered writes, so it's
> > > rejected here.
> > > 
> > > Doing something like this:
> > > 
> > > + if (t->barriers_supported)
> > > +         blk_queue_ordered(q, QUEUE_ORDERED_DRAIN, NULL);
> > > 
> > > somewhere in dm (I stuck it in dm_table_set_restrictions() - almost
> > > certainly the wrong thing to do) did get my dm-linear device to mount
> > > with xfs, w/o xfs complaining that its mount-time barrier tests failed.
> > > 
> > > So what's the right way around this?  What should dm (or md for that
> > > matter) advertise on their queues about ordered-ness?  Should there be
> > > some sort of "QUEUE_ORDERED_PASSTHROUGH" or something to say "this level
> > > doesn't care, ask the next level" or somesuch?  Or should it inherit the
> > > flag from the next level down?  Ideas?
> > > 
> > > Thanks,
> > > -Eric
> > > 
> > > --
> > > dm-devel mailing list
> > > dm-devel@...hat.com
> > > https://www.redhat.com/mailman/listinfo/dm-devel
> > 
> > Hi
> > 
> > This is misdesign in generic bio layer and it should be fixed there. I 
> > think it is blocking barrier support in md-raid1 too. Jens, pls apply the 
> > attached patch.
> > 
> > Mikulas
> > 
> > ----
> > 
> > Move test for not-supported barriers to __make_request.
> > 
> > This test prevents barriers from being dispatched to device mapper
> > and md.
> > 
> > This test is sensible only for drivers that use requests (such as disk
> > drivers), not for drivers that use bios.
> > 
> > It is better to fix it in generic code than to make workaround for it
> > in device mapper and md.
> 
> So you audited any ->make_request_fn style driver and made sure they
> rejected barriers?

I didn't.

If you grep for it, you get:

./arch/powerpc/sysdev/axonram.c:
doesn't reject barriers, but it is not needed, it ends all bios in 
make_request routine

./drivers/block/aoe/aoeblk.c:
* doesn't reject barriers, should be modified to do so

./drivers/block/brd.c
doesn't reject barriers, doesn't need to, ends all bios in make_request

./drivers/block/loop.c:
doesn't reject barriers, it's ok because it doesn't reorder requests

./drivers/block/pktcdvd.c
* doesn't reject barriers, should be modified to do so

./drivers/block/umem.c
* doesn't reject barriers, I don't know if it reorders requests or not.

./drivers/s390/block/xpram.c
doesn't reject barriers, doesn't need, ends bios immediatelly

./drivers/md/raid0.c
rejects barriers

./drivers/md/raid1.c
supports barriers

./drivers/md/raid10.c
rejects barriers

./drivers/md/raid5.c
rejects barriers

./drivers/md/linear.c
rejects barriers

./drivers/md/dm.c
supports barriers partially


Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists