Re: Barrier support in device mapper

Re: Barrier support in device mapper

[Nikanth Karthikesan]

Hi Milan

I was able to talk to Alasdair on Freenode#device-mapper and he is also of the 
opinion full-barrier support is the way to go.

On Thu, Sep 18, 2008 at 12:27 PM, Milan Broz <mbroz@redhat.com> wrote:
> Andi's patch is not complete and I think there can be several problems with 
> it:
>
> - imagine DM device which has barrier support switched on by this simple
> patch and you try to run pvmove on it. How is the barrier request processed
> by underlying devices now?
>
> -> mapping can change online (pvmove, lvextend, lvconvert, ...) to more
> complicated mapping - who reset barrier flag support?
>

I think these things will not create problems.

> - what about stacking devices? Imagine crypto - there is one device per
> table possible under linear target (where you enable barriers by this
> patch).
> dm-crypt will need to implement some queue flushes to properly support
> barriers. Another example - partition mapping over multipath (kpartx), ...
> Are you sure that is it safe with Andi's patch?
> ...

I do not have knowledge of dm-crypt, but yes, dm-crypt might possibly reorder.
Either they should flush the queues or atleast return -EOPNOTSUPP if we need 
to use Andi's patch.

>
> It is dangerous to use that patch IMO, better not support barriers at all
> here. That's why we need something more robust.
>

I understand the possible problems.

> Unfortunately I received _no_ feedback to mentioned RFC barrier patches.

Alasdair said that he will be reviewing/queueing them next week.

Thanks
Nikanth

Re: Barrier support in device mapper

[Andi Kleen]

On Thu, Sep 18, 2008 at 03:46:31PM +0530, Nikanth Karthikesan wrote:
> Hi Milan
> 
> I was able to talk to Alasdair on Freenode#device-mapper and he is also of the 
> opinion full-barrier support is the way to go.
> 
> On Thu, Sep 18, 2008 at 12:27 PM, Milan Broz <mbroz@redhat.com> wrote:
> > Andi's patch is not complete and I think there can be several problems with 
> > it:
> >
> > - imagine DM device which has barrier support switched on by this simple
> > patch and you try to run pvmove on it. How is the barrier request processed
> > by underlying devices now?
> >
> > -> mapping can change online (pvmove, lvextend, lvconvert, ...) to more
> > complicated mapping - who reset barrier flag support?
> >
> 
> I think these things will not create problems.

Correct.

> 
> > - what about stacking devices? Imagine crypto - there is one device per
> > table possible under linear target (where you enable barriers by this
> > patch).
> > dm-crypt will need to implement some queue flushes to properly support
> > barriers. Another example - partition mapping over multipath (kpartx), ...
> > Are you sure that is it safe with Andi's patch?
> > ...
> 
> I do not have knowledge of dm-crypt, but yes, dm-crypt might possibly reorder.
> Either they should flush the queues or atleast return -EOPNOTSUPP if we need 
> to use Andi's patch.

That's already done because it doesn't call dm_table_support_barrier() on
its target. Only the simple remapper does.

All the scenarios you guys are describing (except for pvmove which
just seems to be confused thinking) would happen when the ->barrier_supported
target flag wasn't there, but it really is.

> > It is dangerous to use that patch IMO, better not support barriers at all
> > here. That's why we need something more robust.
> >
> 
> I understand the possible problems.

Sorry, but Milan is just wrong on all of them as far as I know.

-Andi


-- 
ak@linux.intel.com

Barrier support in device mapper

[Nikanth Karthikesan]

Hi Alasdair

What are plans in supporting barriers in device mapper?

Sometime back Andi Kleen posted a patch[1] to support barriers for single-
device dm-devices.

Recently Milan Broz posted an RFC patch-set[2] to implement full-barrier 
support.

Are we going to get full-barrier support soon? If not, I would like to see 
Andi's patch being merged till we get full barrier support ready. I would like 
to know your thoughts.

Thanks
Nikanth Karthikesan
SUSE Labs

[1] http://lkml.org/lkml/2008/2/15/125
[2] http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5941

Re: Barrier support in device mapper

[Milan Broz]

Nikanth Karthikesan wrote:
> Are we going to get full-barrier support soon? If not, I would like to see 
> Andi's patch being merged till we get full barrier support ready. I would like 
> to know your thoughts.

Hi,
Andi's patch is not complete and I think there can be several problems with it:

- imagine DM device which has barrier support switched on by this simple patch and
you try to run pvmove on it. How is the barrier request processed by underlying
devices now?

-> mapping can change online (pvmove, lvextend, lvconvert, ...) to more
complicated mapping - who reset barrier flag support?

- what about stacking devices? Imagine crypto - there is one device per
table possible under linear target (where you enable barriers by this patch).
dm-crypt will need to implement some queue flushes to properly support
barriers. Another example - partition mapping over multipath (kpartx), ...
Are you sure that is it safe with Andi's patch?
...

It is dangerous to use that patch IMO, better not support barriers at all here.
That's why we need something more robust.

Unfortunately I received _no_ feedback to mentioned RFC barrier patches.

Milan
--
mbroz@redhat.com

Re: Barrier support in device mapper

[Andi Kleen]

On Thu, Sep 18, 2008 at 08:57:02AM +0200, Milan Broz wrote:
> - imagine DM device which has barrier support switched on by this simple patch and
> you try to run pvmove on it. How is the barrier request processed by underlying
> devices now?

I don't see how a barrier can go through incorrectly even in this scenario. The pvmove
will always be handled by the remapper and then remapper always checks
underlying single device and barrier supported and rejects the barrier
as needed. 

After a request passed the remapper to extent cannot be moved elsewhere again
because it's already in flight on the underlying device queues.

Please describe the scenario in more detail that you think will break.

> 
> -> mapping can change online (pvmove, lvextend, lvconvert, ...) to more
> complicated mapping - who reset barrier flag support?
> 
> - what about stacking devices? Imagine crypto - there is one device per
> table possible under linear target (where you enable barriers by this patch).
> dm-crypt will need to implement some queue flushes to properly support
> barriers.

Yes dm_crypt needs more work for barriers (I have an experimental but not
quite right patch here for that), but my other patch doesn't enable 
barriers on dm_crypt anyways. dm_crypt doesn't call dm_barrier_supported() 
on its target ever, so there won't be any barriers for it.

> Another example - partition mapping over multipath (kpartx), ...

dm-mpath doesn't call dm_barrier_supported() either. Did you
really read my patch?  It sounds like you're ranting about something
else, but not my patch.

> Are you sure that is it safe with Andi's patch?

I think it is.

> That's why we need something more robust.
> 
> Unfortunately I received _no_ feedback to mentioned RFC barrier patches.

What patches?  Pointer please.

-Andi

Re: Barrier support in device mapper

[Milan Broz]

Andi Kleen wrote:
> On Thu, Sep 18, 2008 at 08:57:02AM +0200, Milan Broz wrote:
>> - imagine DM device which has barrier support switched on by this simple patch and
>> you try to run pvmove on it. How is the barrier request processed by underlying
>> devices now?
> 
> I don't see how a barrier can go through incorrectly even in this scenario. The pvmove
> will always be handled by the remapper and then remapper always checks
> underlying single device and barrier supported and rejects the barrier
> as needed. 

Well, it depends what we expect from barrier implementation.
I work with the idea that if block device support barriers,
it should support them in all situations.

You patch test every single request, so the same request can succeed,
minute later the same request can return NOTSUPP because mapping changed.
So if this is what you want, you are right - it works.

The pvmove example:
1) one disk, linear mapping -> barriers supported
2) user run pvmove to another disk -> all barriers are rejected during the move
3) pvmove finished -> barriers are supported again

That's probably fine (I expect barrier fs code support this correctly).
But it is workaround, not real barrier support in device-mapper.

>> - what about stacking devices? Imagine crypto - there is one device per
>> table possible under linear target (where you enable barriers by this patch).
>> dm-crypt will need to implement some queue flushes to properly support
>> barriers.
> 
> Yes dm_crypt needs more work for barriers (I have an experimental but not
> quite right patch here for that), but my other patch doesn't enable 
> barriers on dm_crypt anyways. dm_crypt doesn't call dm_barrier_supported() 
> on its target ever, so there won't be any barriers for it.
> 
>> Another example - partition mapping over multipath (kpartx), ...
> 
> dm-mpath doesn't call dm_barrier_supported() either. Did you
> really read my patch?  It sounds like you're ranting about something
> else, but not my patch.

Sure - your patch enabled it only for linear target. So the request will fail
somewhere down the stack - in mpath/crypt/whatever target.

That wass not against your patch - I just want more generic solution
(and and wrote some simple implementation already).
And these workarounds mostly delay the real implementation...

I want barriers for all targets - use some core functionality and process
only special situation in targets. See my comments in link below.

>> Unfortunately I received _no_ feedback to mentioned RFC barrier patches.
> 
> What patches?  Pointer please.

The mail I replied to contains links to all mentioned patches:

> [1] http://lkml.org/lkml/2008/2/15/125
> [2] http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5941

Milan

Re: Barrier support in device mapper

[Andi Kleen]

On Thu, Sep 18, 2008 at 06:08:55PM +0200, Milan Broz wrote:
> Andi Kleen wrote:
> > On Thu, Sep 18, 2008 at 08:57:02AM +0200, Milan Broz wrote:
> >> - imagine DM device which has barrier support switched on by this simple patch and
> >> you try to run pvmove on it. How is the barrier request processed by underlying
> >> devices now?
> > 
> > I don't see how a barrier can go through incorrectly even in this scenario. The pvmove
> > will always be handled by the remapper and then remapper always checks
> > underlying single device and barrier supported and rejects the barrier
> > as needed. 
> 
> Well, it depends what we expect from barrier implementation.
> I work with the idea that if block device support barriers,
> it should support them in all situations.

It does.

> 
> You patch test every single request, so the same request can succeed,
> minute later the same request can return NOTSUPP because mapping changed.
> So if this is what you want, you are right - it works.
> 
> The pvmove example:
> 1) one disk, linear mapping -> barriers supported
> 2) user run pvmove to another disk -> all barriers are rejected during the move
> 3) pvmove finished -> barriers are supported again
> 
> That's probably fine (I expect barrier fs code support this correctly).
> But it is workaround, not real barrier support in device-mapper.

It works for 95+% of the users that only have a single disk.
That is all my patch was attempting: support the single disk case.

Yes I know there could be more done for multiple disks or dm crypt
(although I think to do it well for multiple disks needs behaviour
changes in the barriers), but that's orthogonal and shouldn't delay
the single disk case patch.


> 
> That wass not against your patch - I just want more generic solution
> (and and wrote some simple implementation already).

Does solve the single disk case?  If it passes the barriers
through similar to my patch in the single disk case they would
be fine for me.

Although I don't know if your patches would be ready for a .28 merge.
I think my patch is. So if your patches would need more work
it would be quite reasonable to target the simple patch for .28
and do the more complex stuff later.

> And these workarounds mostly delay the real implementation...

I don't know why you can call it a workaround. It's a perfectly fine
design to handle barriers on single disk. And no, complexity is not
always the goal.

I also don't see how it delays more complex solutions. If you have
a more sophisticated scheme ready you can just do it on top of that
patch. It's not that it's an extremly complicated patch that makes
it hard to do other patches on top of it :)

> 
> >> Unfortunately I received _no_ feedback to mentioned RFC barrier patches.
> > 
> > What patches?  Pointer please.
> 
> The mail I replied to contains links to all mentioned patches:
> 
> > [1] http://lkml.org/lkml/2008/2/15/125
> > [2] http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5941

That's just a description, where are the patches?

Are they equivalent in the single disk case to my patch?

-Andi

Re: Barrier support in device mapper

[Milan Broz]

Andi Kleen wrote:
>> The mail I replied to contains links to all mentioned patches:
>>
>>> [1] http://lkml.org/lkml/2008/2/15/125
>>> [2] http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5941
> 
> That's just a description, where are the patches?

http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5942
http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5943
http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5944
http://article.gmane.org/gmane.linux.kernel.device-mapper.devel/5945

> Are they equivalent in the single disk case to my patch?
It transforms barrier to normal bio with following zero-size barrier bio.
For the one disk case effect is the same of course.

Milan

Re: Barrier support in device mapper

[Andi Kleen]

> > Are they equivalent in the single disk case to my patch?
> It transforms barrier to normal bio with following zero-size barrier bio.
> For the one disk case effect is the same of course.

Ok fine assuming they work. Will they be ready for the 2.6.28 merge window?

If not I would still prefer my patch for .28

-Andi