2006-01-20 21:18:08

by Alasdair G Kergon

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

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

Signed-Off-By: Alasdair G Kergon <[email protected]>

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;
}


2006-01-23 05:41:36

by Andrew Morton

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

Alasdair G Kergon <[email protected]> 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?

2006-01-23 15:57:09

by Lars Marowsky-Bree

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

On 2006-01-22T21:41:11, Andrew Morton <[email protected]> wrote:

> Alasdair G Kergon <[email protected]> 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"

2006-01-23 17:02:03

by Alasdair G Kergon

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

On Sun, Jan 22, 2006 at 09:41:11PM -0800, Andrew Morton wrote:
> Alasdair G Kergon <[email protected]> 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
--
[email protected]

2006-01-23 21:15:21

by Andrew Morton

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

Lars Marowsky-Bree <[email protected]> wrote:
>
> On 2006-01-22T21:41:11, Andrew Morton <[email protected]> wrote:
>
> > Alasdair G Kergon <[email protected]> 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?

2006-01-25 20:55:26

by Alasdair G Kergon

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

On Mon, Jan 23, 2006 at 01:14:46PM -0800, Andrew Morton wrote:
> Lars Marowsky-Bree <[email protected]> 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
--
[email protected]