| Message ID | 49DC92CC.30708@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Alasdair Kergon |
| Headers | show |
On Wed, Apr 8, 2009 at 05:04, Peter Rajnoha <prajnoha@redhat.com> wrote: > this is a kernel side patch that adds support for cookie variables. > A cookie is a random unsigned 32 bit value generated in userspace > and passed into the DM through ioctl (reusing the event_nr field to > avoid the change in the interface). These values are sent back in > CHANGE and REMOVE uevents. This way we can easily pair a process > waiting for udev's rule completion and the udev event itself. That sounds useful. I always thought we would return the driver core generated seqnum with the ioctl, so you could watch the udev event queue to see the events being handled. But if you are going to listen to the udev events, the cookie sounds like a nice solution. > Also, > we can use the cookie value to group the actions and related uevents > together, so we know which uevents belong to actions done on one > DM tree. If you would use the same cookie for several actions, how do you track, that udev has finished the work when you expect an arbitrary number of events with the same cookie? Thanks, Kay -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Kay Sievers wrote: > If you would use the same cookie for several actions, how do you > track, that udev has finished the work when you expect an arbitrary > number of events with the same cookie? Well, I use a simple semaphore in userspace waiting for zero. So for every action I just increment the semaphore and when the cookie comes back from uevent, I decrement it (I use this cookie value as the semaphore's identifier as well). So the number of uevents I'm expecting is not arbitrary from this perspective. I just need to fire off all the actions and then at some point later need to be sure that all the udev rules were executed (so I'm sure all the nodes are created) and so I can continue. Peter -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Apr 08, 2009 at 05:49:11AM -0700, Kay Sievers wrote: > I always thought we would return the driver core generated seqnum with > the ioctl, When Peter tried to make that work it needed more and more hacks piled onto it and instead he came up with this cookie method which seems to satisfy all our requirements with a minimum amount of change, fingers crossed. E.g. A problem with the other approach was: lvm2 userspace doesn't know the seqnum till after the uevent has fired and udev could complete processing before the lvm2 code has set itself up to wait for that seqnum so you have to build extra interlocking into the process making it much more complex. If you wait for all events up to a given number you could get stuck behind some unrelated event on an unresponsive device, with is also unsatisfactory. Alasdair
On Wed, Apr 8, 2009 at 05:58, Peter Rajnoha <prajnoha@redhat.com> wrote: > Kay Sievers wrote: >> If you would use the same cookie for several actions, how do you >> track, that udev has finished the work when you expect an arbitrary >> number of events with the same cookie? > > Well, I use a simple semaphore in userspace waiting for zero. So for > every action I just increment the semaphore and when the cookie comes back > from uevent, I decrement it (I use this cookie value as the semaphore's > identifier as well). So the number of uevents I'm expecting is not > arbitrary from this perspective. I just need to fire off all the actions > and then at some point later need to be sure that all the udev rules > were executed (so I'm sure all the nodes are created) and so I can continue. Sure, sounds good. I just read "we can use the cookie value to group the actions" as you would want to use the same value for several actions. Thanks, Kay -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Apr 8, 2009 at 06:20, Alasdair G Kergon <agk@redhat.com> wrote: > On Wed, Apr 08, 2009 at 05:49:11AM -0700, Kay Sievers wrote: >> I always thought we would return the driver core generated seqnum with >> the ioctl, > > When Peter tried to make that work it needed more and more hacks > piled onto it and instead he came up with this cookie method which seems > to satisfy all our requirements with a minimum amount of change, fingers > crossed. > > E.g. A problem with the other approach was: lvm2 userspace doesn't know the > seqnum till after the uevent has fired and udev could complete processing > before the lvm2 code has set itself up to wait for that seqnum so you have to > build extra interlocking into the process making it much more complex. Â If you > wait for all events up to a given number you could get stuck behind some > unrelated event on an unresponsive device, with is also unsatisfactory. Yeah, sounds sensible, and like a nice solution to the problem if you are able to watch the events udev sends out. Thanks, Kay -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c index 823ceba..6ad924a 100644 --- a/drivers/md/dm-ioctl.c +++ b/drivers/md/dm-ioctl.c @@ -276,7 +276,7 @@ retry: up_write(&_hash_lock); } -static int dm_hash_rename(const char *old, const char *new) +static int dm_hash_rename(uint32_t cookie, const char *old, const char *new) { char *new_name, *old_name; struct hash_cell *hc; @@ -333,6 +333,7 @@ static int dm_hash_rename(const char *old, const char *new) dm_table_put(table); } + dm_set_cookie(hc->md, cookie); dm_kobject_uevent(hc->md); dm_put(hc->md); @@ -678,9 +679,11 @@ static int dev_remove(struct dm_ioctl *param, size_t param_size) return r; } + dm_set_cookie(md, param->event_nr); + __hash_remove(hc); - up_write(&_hash_lock); dm_put(md); + up_write(&_hash_lock); param->data_size = 0; return 0; } @@ -715,7 +718,7 @@ static int dev_rename(struct dm_ioctl *param, size_t param_size) return r; param->data_size = 0; - return dm_hash_rename(param->name, new_name); + return dm_hash_rename(param->event_nr, param->name, new_name); } static int dev_set_geometry(struct dm_ioctl *param, size_t param_size) @@ -814,8 +817,6 @@ static int do_resume(struct dm_ioctl *param) hc->new_map = NULL; param->flags &= ~DM_INACTIVE_PRESENT_FLAG; - up_write(&_hash_lock); - /* Do we need to load a new map ? */ if (new_map) { /* Suspend if it isn't already suspended */ @@ -839,13 +840,18 @@ static int do_resume(struct dm_ioctl *param) set_disk_ro(dm_disk(md), 1); } + dm_set_cookie(md, param->event_nr); + if (dm_suspended(md)) r = dm_resume(md); + else + dm_kobject_uevent(md); if (!r) r = __dev_status(md, param); dm_put(md); + up_write(&_hash_lock); return r; } diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c index a2a45e6..2c9b0db 100644 --- a/drivers/md/dm-sysfs.c +++ b/drivers/md/dm-sysfs.c @@ -95,5 +95,9 @@ int dm_sysfs_init(struct mapped_device *md) */ void dm_sysfs_exit(struct mapped_device *md) { + char *envp[] = {dm_get_cookie(md), NULL}; + struct kobject *kobj = &disk_to_dev(dm_disk(md))->kobj; + + kobject_uevent_env(kobj, KOBJ_REMOVE, envp); kobject_put(dm_kobject(md)); } diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 8a994be..52bee12 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -155,6 +155,7 @@ struct mapped_device { atomic_t uevent_seq; struct list_head uevent_list; spinlock_t uevent_lock; /* Protect access to uevent_list */ + char cookie[24]; /* * freeze/thaw support require holding onto a super block @@ -1727,7 +1728,9 @@ out: *---------------------------------------------------------------*/ void dm_kobject_uevent(struct mapped_device *md) { - kobject_uevent(&disk_to_dev(md->disk)->kobj, KOBJ_CHANGE); + char *envp[] = {dm_get_cookie(md), NULL}; + + kobject_uevent_env(&disk_to_dev(md->disk)->kobj, KOBJ_CHANGE, envp); } uint32_t dm_next_uevent_seq(struct mapped_device *md) @@ -1764,6 +1767,16 @@ struct gendisk *dm_disk(struct mapped_device *md) return md->disk; } +void dm_set_cookie(struct mapped_device *md, uint32_t cookie) +{ + snprintf(md->cookie, sizeof(md->cookie), "COOKIE=%u", (unsigned) cookie); +} + +char *dm_get_cookie(struct mapped_device *md) +{ + return md->cookie; +} + struct kobject *dm_kobject(struct mapped_device *md) { return &md->kobj; diff --git a/drivers/md/dm.h b/drivers/md/dm.h index a31506d..0c6dbaa 100644 --- a/drivers/md/dm.h +++ b/drivers/md/dm.h @@ -93,6 +93,8 @@ int dm_open_count(struct mapped_device *md); int dm_lock_for_deletion(struct mapped_device *md); void dm_kobject_uevent(struct mapped_device *md); +void dm_set_cookie(struct mapped_device *md, uint32_t cookie); +char *dm_get_cookie(struct mapped_device *md); int dm_kcopyd_init(void); void dm_kcopyd_exit(void); diff --git a/include/linux/dm-ioctl.h b/include/linux/dm-ioctl.h index 48e44ee..dcf0192 100644 --- a/include/linux/dm-ioctl.h +++ b/include/linux/dm-ioctl.h @@ -123,7 +123,8 @@ struct dm_ioctl { __u32 target_count; /* in/out */ __s32 open_count; /* out */ __u32 flags; /* in/out */ - __u32 event_nr; /* in/out */ + __u32 event_nr; /* in/out + * used both for event numbers and cookie values */ __u32 padding; __u64 dev; /* in/out */ @@ -256,9 +257,9 @@ enum { #define DM_DEV_SET_GEOMETRY _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl) #define DM_VERSION_MAJOR 4 -#define DM_VERSION_MINOR 14 +#define DM_VERSION_MINOR 15 #define DM_VERSION_PATCHLEVEL 0 -#define DM_VERSION_EXTRA "-ioctl (2008-04-23)" +#define DM_VERSION_EXTRA "-ioctl (2009-04-01)" /* Status bits */ #define DM_READONLY_FLAG (1 << 0) /* In/Out */