[PATCH 6/9] device-mapper snapshot: barriers not supported

[PATCH 6/9] device-mapper snapshot: barriers not supported

[Alasdair G Kergon]

The snapshot and origin targets are incapable of handling barriers and 
need to indicate this.

Signed-Off-By: Alasdair G Kergon <agk@redhat.com>

Index: linux-2.6.16-rc1/drivers/md/dm-snap.c
===================================================================
--- linux-2.6.16-rc1.orig/drivers/md/dm-snap.c
+++ linux-2.6.16-rc1/drivers/md/dm-snap.c
@@ -792,6 +792,9 @@ static int snapshot_map(struct dm_target
 	if (!s->valid)
 		return -EIO;
 
+	if (unlikely(bio_barrier(bio)))
+		return -EOPNOTSUPP;
+
 	/*
 	 * Write to snapshot - higher level takes care of RW/RO
 	 * flags so we should only get this if we are
@@ -1058,6 +1061,9 @@ static int origin_map(struct dm_target *
 	struct dm_dev *dev = (struct dm_dev *) ti->private;
 	bio->bi_bdev = dev->bdev;
 
+	if (unlikely(bio_barrier(bio)))
+		return -EOPNOTSUPP;
+
 	/* Only tell snapshots if this is a write */
 	return (bio_rw(bio) == WRITE) ? do_origin(dev, bio) : 1;
 }

Re: [PATCH 6/9] device-mapper snapshot: barriers not supported

[Andrew Morton]

Alasdair G Kergon <agk@redhat.com> wrote:
>
> The snapshot and origin targets are incapable of handling barriers and 
>  need to indicate this.
> 
> ...
>   
>  +	if (unlikely(bio_barrier(bio)))
>  +		return -EOPNOTSUPP;
>  +

And what was happening if people _were_ sending such BIOs down?  Did it all
appear to work correctly?  If so, will this change cause
currently-apparently-working setups to stop working?

Re: [PATCH 6/9] device-mapper snapshot: barriers not supported

[Lars Marowsky-Bree]

On 2006-01-22T21:41:11, Andrew Morton <akpm@osdl.org> wrote:

> Alasdair G Kergon <agk@redhat.com> wrote:
> >
> > The snapshot and origin targets are incapable of handling barriers and 
> >  need to indicate this.
> > 
> > ...
> >   
> >  +	if (unlikely(bio_barrier(bio)))
> >  +		return -EOPNOTSUPP;
> >  +
> 
> And what was happening if people _were_ sending such BIOs down?  Did it all
> appear to work correctly?  If so, will this change cause
> currently-apparently-working setups to stop working?

Filesystems basically disable using barriers on a device which doesn't
support them, which is indicated by -EOPNOTSUPP. Barriers are allowed to
fail in such fashion.

Now the interesting question is what happens when barriers are suddenly
verboten on a stack which used to support them - because the new mapping
doesn't support it _anymore_. Hrm. _Should_ work, but probably not
tested much ;-)


Sincerely,
    Lars Marowsky-Brée

-- 
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business	 -- Charles Darwin
"Ignorance more frequently begets confidence than does knowledge"

Re: [PATCH 6/9] device-mapper snapshot: barriers not supported

[Alasdair G Kergon]

On Sun, Jan 22, 2006 at 09:41:11PM -0800, Andrew Morton wrote:
> Alasdair G Kergon <agk@redhat.com> wrote:
> > The snapshot and origin targets are incapable of handling barriers and 
> >  need to indicate this.

> And what was happening if people _were_ sending such BIOs down?  Did it all
> appear to work correctly?  

The snapshot target ignores the barrier and in some circumstances I/O can be
reordered in ways that are meant to be prevented by the barrier.

> If so, will this change cause
> currently-apparently-working setups to stop working?

As Lars pointed out, filesystems should already cope with -EOPNOTSUPP 
transparently, and it would be sensible for any out-of-tree users to 
do likewise.

Alasdair
-- 
agk@redhat.com

Re: [PATCH 6/9] device-mapper snapshot: barriers not supported

[Andrew Morton]

Lars Marowsky-Bree <lmb@suse.de> wrote:
>
> On 2006-01-22T21:41:11, Andrew Morton <akpm@osdl.org> wrote:
> 
> > Alasdair G Kergon <agk@redhat.com> wrote:
> > >
> > > The snapshot and origin targets are incapable of handling barriers and 
> > >  need to indicate this.
> > > 
> > > ...
> > >   
> > >  +	if (unlikely(bio_barrier(bio)))
> > >  +		return -EOPNOTSUPP;
> > >  +
> > 
> > And what was happening if people _were_ sending such BIOs down?  Did it all
> > appear to work correctly?  If so, will this change cause
> > currently-apparently-working setups to stop working?
> 
> Filesystems basically disable using barriers on a device which doesn't
> support them, which is indicated by -EOPNOTSUPP. Barriers are allowed to
> fail in such fashion.
> 
> Now the interesting question is what happens when barriers are suddenly
> verboten on a stack which used to support them - because the new mapping
> doesn't support it _anymore_. Hrm. _Should_ work, but probably not
> tested much ;-)
> 

I don't understand that, sorry.

My concern is: has the above change any potential to cause
currently-working setups to stop working?

Re: [PATCH 6/9] device-mapper snapshot: barriers not supported

[Alasdair G Kergon]

On Mon, Jan 23, 2006 at 01:14:46PM -0800, Andrew Morton wrote:
> Lars Marowsky-Bree <lmb@suse.de> wrote:
> > Now the interesting question is what happens when barriers are suddenly
> > verboten on a stack which used to support them - because the new mapping
> > doesn't support it _anymore_. Hrm. _Should_ work, but probably not
> > tested much ;-)

> I don't understand that, sorry.
 
> My concern is: has the above change any potential to cause
> currently-working setups to stop working?
 
I trust not.  Various things don't support barriers yet, so code 
shouldn't assume that they are supported.  Where they are supported,
they allow for optimisation.


For example, jbd detects the error and retries without them.

  fs/jbd/commit.c: static int journal_write_commit_record(

        if (ret == -EOPNOTSUPP && barrier_done) {
		... 
                printk(KERN_WARNING
                        "JBD: barrier-based sync failed on %s - "
                        "disabling barriers\n",
                        bdevname(journal->j_dev, b));
		...
		/* And try again, without the barrier */
		...

Alasdair
-- 
agk@redhat.com