FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 12-11-2007, 03:25 PM
Alasdair G Kergon
 
Default improve atomicity of device creation

On Tue, Dec 11, 2007 at 04:08:36PM +0000, Scott James Remnant wrote:
> This is a patch we've written and applied in Ubuntu to improve the
> atomicity of devmapper device creation somewhat, and avoid races with
> udev.

BTW I won't be applying this upstream - these nodes are designed to be
managed by libdevmapper not udev so it's not relevant. I'm awaiting
patches that have been promised to an agreed design that will provide
for correct libdevmapper/udev integration without races/deadlocks etc.

Alasdair
--
agk@redhat.com

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 04:03 PM
Alasdair G Kergon
 
Default improve atomicity of device creation

On Tue, Dec 11, 2007 at 04:42:13PM +0000, Scott James Remnant wrote:
> What is that agreed design?

In simple terms:

- udev takes over full responsibility for creating nodes
[this will probably be a ./configure option]

- udev provides interface we use to wait until it has finished
processing all the outstanding requests we sent it.

Key point is that our udev requests appear in batches, then we wait for
the batch to complete, as when the system is under memory pressure, udev
userspace may find itself blocked processing the first of the batch
until just before we issue the wait at the end of the batch.
[Note how libdevmapper today pushes all the requests onto an internal
stack and processes them all together at the end.]

Alasdair
--
agk@redhat.com

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 04:18 PM
Scott James Remnant
 
Default improve atomicity of device creation

On Tue, 2007-12-11 at 17:03 +0000, Alasdair G Kergon wrote:

> On Tue, Dec 11, 2007 at 04:42:13PM +0000, Scott James Remnant wrote:
> > What is that agreed design?
>
> In simple terms:
>
> - udev takes over full responsibility for creating nodes
> [this will probably be a ./configure option]
>
Excellent, this is what we've wanted for a while.

> - udev provides interface we use to wait until it has finished
> processing all the outstanding requests we sent it.
>
What's this interface?

I had a proposed patch that made udev write its sequence number to the
kernel, and thus allowed any kobject add event to have a "bottom half"
that could happen when udev had finished. The idea was that the
existing ioctls could then just block.

The other patch we tried a couple of releases ago was have devmapper
spin until udev had caught up.

Scott
--
Scott James Remnant
scott@ubuntu.com
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 04:35 PM
"Kay Sievers"
 
Default improve atomicity of device creation

On Dec 11, 2007 5:08 PM, Scott James Remnant <scott@ubuntu.com> wrote:
> This is a patch we've written and applied in Ubuntu to improve the
> atomicity of devmapper device creation somewhat, and avoid races with
> udev.

Without synchronization between libdevmapper and udev, you can't
predict which one will win in creating the node, right?
So this patch makes is more likely to create a valid device node, but
it can still happen, that libdevmapper replaces the udev nodes, which
is not what we want, right?

Kay

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 04:40 PM
Scott James Remnant
 
Default improve atomicity of device creation

On Tue, 2007-12-11 at 18:35 +0100, Kay Sievers wrote:

> On Dec 11, 2007 5:08 PM, Scott James Remnant <scott@ubuntu.com> wrote:
> > This is a patch we've written and applied in Ubuntu to improve the
> > atomicity of devmapper device creation somewhat, and avoid races with
> > udev.
>
> Without synchronization between libdevmapper and udev, you can't
> predict which one will win in creating the node, right?
> So this patch makes is more likely to create a valid device node, but
> it can still happen, that libdevmapper replaces the udev nodes, which
> is not what we want, right?
>
No, the patch is slightly skewed so that udev always wins. udev will
always adopt an existing device node, adjusting permissions as
necessary. The patch makes devmapper "back off" if the device node
exists.

So you end up with either:

1) devmapper creates device node
2) udev adopts device node and sets permissions

or:

1) udev creates device node and sets permissions
2) devmapper no-ops since it already exists

Scott
--
Scott James Remnant
scott@ubuntu.com
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 04:51 PM
"Kay Sievers"
 
Default improve atomicity of device creation

On Dec 11, 2007 6:18 PM, Scott James Remnant <scott@ubuntu.com> wrote:
> On Tue, 2007-12-11 at 17:03 +0000, Alasdair G Kergon wrote:
>
> > On Tue, Dec 11, 2007 at 04:42:13PM +0000, Scott James Remnant wrote:
> > > What is that agreed design?
> >
> > In simple terms:
> >
> > - udev takes over full responsibility for creating nodes
> > [this will probably be a ./configure option]
> >
> Excellent, this is what we've wanted for a while.

Sounds fine.

> > - udev provides interface we use to wait until it has finished
> > processing all the outstanding requests we sent it.
> >
> What's this interface?
>
> I had a proposed patch that made udev write its sequence number to the
> kernel, and thus allowed any kobject add event to have a "bottom half"
> that could happen when udev had finished. The idea was that the
> existing ioctls could then just block.

You want to block processes (doing ioctl) until udev has finished? Like a
kernel userspace transaction? Where could you see which requests you
will need to fulfill to continue the hanging processes?

It could work, but if something goes wrong, it seems pretty hard to recover
from such a failure. Just remember how firmware loading works.

> The other patch we tried a couple of releases ago was have devmapper
> spin until udev had caught up.

The currently running udev events are in: /dev/.udev/queue/<seqnum>. The last
seqnum that arrived in the udev daemon is: /dev/.udev/uevent_seqnum. So udev's
state is exported.

We can make the kernel's kobject_uevent() return the generated seqnum to
the caller. A devmapper ioctl, an existing one to extend, or a new one like
"create node" can return the seqnum to libdevmapper. Libdevmapper then
checks and spins until /dev/.udev/uevent_seqnum has seen the seqnum,
and /dev/.udev/queue/<seqnum> does not exist.

Kay

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 04:53 PM
Alasdair G Kergon
 
Default improve atomicity of device creation

On Tue, Dec 11, 2007 at 05:40:28PM +0000, Scott James Remnant wrote:
> 1) udev creates device node and sets permissions
> 2) devmapper no-ops since it already exists

...because libdevmapper has no notion that something other than itself
could have created it. That's a bug, but provided the proper udev
integration isn't much longer in coming, I shan't bother to fix it.

Alasdair
--
agk@redhat.com

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 05:03 PM
"Kay Sievers"
 
Default improve atomicity of device creation

On Dec 11, 2007 6:40 PM, Scott James Remnant <scott@ubuntu.com> wrote:
>
> On Tue, 2007-12-11 at 18:35 +0100, Kay Sievers wrote:
>
> > On Dec 11, 2007 5:08 PM, Scott James Remnant <scott@ubuntu.com> wrote:
> > > This is a patch we've written and applied in Ubuntu to improve the
> > > atomicity of devmapper device creation somewhat, and avoid races with
> > > udev.
> >
> > Without synchronization between libdevmapper and udev, you can't
> > predict which one will win in creating the node, right?
> > So this patch makes is more likely to create a valid device node, but
> > it can still happen, that libdevmapper replaces the udev nodes, which
> > is not what we want, right?
> >
> No, the patch is slightly skewed so that udev always wins. udev will
> always adopt an existing device node, adjusting permissions as
> necessary. The patch makes devmapper "back off" if the device node
> exists.
>
> So you end up with either:
>
> 1) devmapper creates device node
> 2) udev adopts device node and sets permissions
>
> or:
>
> 1) udev creates device node and sets permissions
> 2) devmapper no-ops since it already exists

There is a window between stat() and rename() in libdevmapper,
how can you konw that "udev always wins"?

Kay

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 05:08 PM
Alasdair G Kergon
 
Default improve atomicity of device creation

On Tue, Dec 11, 2007 at 06:51:31PM +0100, Kay Sievers wrote:
> We can make the kernel's kobject_uevent() return the generated seqnum to
> the caller.

Is 32 bits enough?

We added 'uint32_t padding;' recently which we I reckon we could now use for
this.

dm_kobject_uevent() and alloc_dev() could keep a new field in
struct mapped_device up-to-date, and a new function could return its
value to dm-ioctl.c code to place into a renamed 'padding' field
before returning to userspace?

Alasdair
--
agk@redhat.com

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 12-11-2007, 05:25 PM
Kay Sievers
 
Default improve atomicity of device creation

On Tue, 2007-12-11 at 18:08 +0000, Alasdair G Kergon wrote:
> On Tue, Dec 11, 2007 at 06:51:31PM +0100, Kay Sievers wrote:
> > We can make the kernel's kobject_uevent() return the generated seqnum to
> > the caller.
>
> Is 32 bits enough?

In the kernel it is 64 bit. Udev also uses 64bit number, but it does not
care about the value these days (netlink is never out of order,
unlike /sbin/hotplug). Only the udev queue export would break if the
number wraps around and duplicates are generated in a window of 180
seconds (the event process kills itself after that time).

So we would need to strip the upper 32 bit of the numbers we see from
udev, and handle the case where the 32 bit number wraps around.

> We added 'uint32_t padding;' recently which we I reckon we could now use for
> this.
>
> dm_kobject_uevent() and alloc_dev() could keep a new field in
> struct mapped_device up-to-date, and a new function could return its
> value to dm-ioctl.c code to place into a renamed 'padding' field
> before returning to userspace?

Yes, we would return the kernel generated seqnum to the libdevmapper
ioctl.

Should we create an additional new iocl to request the device node
creation, or add that call to an existing one?

Could it be that we want to pass more properties to the event, which are
added as environment keys?

Kay

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 07:33 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org