udevd: serialize device chain event sequence

udevd: serialize device chain event sequence

[Kay Sievers]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

Any objections against the serialization of the event sequence of a
chain of devices. Currently udevd delays only events for the same
DEVPATH.

Example of an "add" event sequence:
  /block/sda
  /block/sda/sda1

With this change, we make sure, that the udev process handling /block/sda
has finished its work (waited for all attributes, created the node) before
we fork the udev event for /block/sda/sda1. This way the event for sda1 can
be sure, that the node for the main device is already created (may be
useful for disk labels).

The main motivation to do this is the program execution of the dev.d/
and hotplug.d/ directory. If we don't wait for the parent event to exit,
we can't be sure that the executed scripts are run in the right order.

It will not affect any parallel device handling, only the sequence of the
devices directory chain is serialized. The 10.000 disks plugged in will still
run as parallel events :)

Thanks,
Kay

[-- Attachment #2: udevd-serialize-event-sequence-01.patch --]
[-- Type: text/plain, Size: 1248 bytes --]

===== udevd.c 1.53 vs edited =====
--- 1.53/udevd.c	2004-11-28 05:56:22 +01:00
+++ edited/udevd.c	2004-12-09 00:04:57 +01:00
@@ -151,16 +151,24 @@
 	}
 }
 
-/* returns already running task with devpath */
+/* returns still running task for the same event sequence */
 static struct hotplug_msg *running_with_devpath(struct hotplug_msg *msg)
 {
 	struct hotplug_msg *loop_msg;
+	int i;
+
 	list_for_each_entry(loop_msg, &running_list, list) {
 		if (loop_msg->devpath == NULL || msg->devpath == NULL)
 			continue;
 
-		if (strcmp(loop_msg->devpath, msg->devpath) == 0)
-			return loop_msg;
+		/* is one DEVPATH a complete part of the other? */
+		for (i = 0; i < DEVPATH_SIZE; i++) {
+			if (loop_msg->devpath[i] == '\0' || msg->devpath[i] == '\0')
+				return loop_msg;
+
+			if (loop_msg->devpath[i] != msg->devpath[i])
+				break;
+		}
 	}
 
 	return NULL;
@@ -181,8 +189,8 @@
 			udev_run(loop_msg);
 			dbg("moved seq %llu to running list", loop_msg->seqnum);
 		} else {
-			dbg("delay seq %llu, cause seq %llu already working on '%s'",
-				loop_msg->seqnum, msg->seqnum, msg->devpath);
+			dbg("delay seq %llu (%s), cause seq %llu (%s) is still running",
+			    loop_msg->seqnum, loop_msg->devpath, msg->seqnum, msg->devpath);
 		}
 	}
 }

Re: udevd: serialize device chain event sequence

[David Brownell]

On Wednesday 08 December 2004 3:13 pm, Kay Sievers wrote:
> Any objections against the serialization of the event sequence of a
> chain of devices. Currently udevd delays only events for the same
> DEVPATH.

I'm amused ... that's what the original hotplug code achieved.
Thing is, now serialization would be done in userspace, rather
than offering new and fun paths to kernel deadlock nirvana!  :)


> Example of an "add" event sequence:
>   /block/sda
>   /block/sda/sda1
> 
> With this change, we make sure, that the udev process handling /block/sda
> has finished its work (waited for all attributes, created the node) before
> we fork the udev event for /block/sda/sda1. This way the event for sda1 can
> be sure, that the node for the main device is already created (may be
> useful for disk labels).

But the "add" for the underlying hardware (maybe USB) would
not be serialized.


> The main motivation to do this is the program execution of the dev.d/
> and hotplug.d/ directory. If we don't wait for the parent event to exit,
> we can't be sure that the executed scripts are run in the right order.

Could that argument apply to the underlying hardware, too?

- Dave



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevd: serialize device chain event sequence

[Kay Sievers]

On Wed, 2004-12-08 at 19:07 -0800, David Brownell wrote: 
> On Wednesday 08 December 2004 3:13 pm, Kay Sievers wrote:
> > Any objections against the serialization of the event sequence of a
> > chain of devices. Currently udevd delays only events for the same
> > DEVPATH.
> 
> I'm amused ... that's what the original hotplug code achieved.
> Thing is, now serialization would be done in userspace, rather
> than offering new and fun paths to kernel deadlock nirvana!  :)

Yeah, the same applies to current userspace code, like the one we have
in HAL :) 

> > Example of an "add" event sequence:
> >   /block/sda
> >   /block/sda/sda1
> > 
> > With this change, we make sure, that the udev process handling /block/sda
> > has finished its work (waited for all attributes, created the node) before
> > we fork the udev event for /block/sda/sda1. This way the event for sda1 can
> > be sure, that the node for the main device is already created (may be
> > useful for disk labels).
> 
> But the "add" for the underlying hardware (maybe USB) would
> not be serialized.

Right, just the stream of events for the /sys/devices/-device is
serialized, but in another event sequence. The class/block device
carries from kernel 2.6.10+ on the PHYSDEVPATH variable in the
environment, which makes it easy to know what to wait for. 

> > The main motivation to do this is the program execution of the dev.d/
> > and hotplug.d/ directory. If we don't wait for the parent event to exit,
> > we can't be sure that the executed scripts are run in the right order.
> 
> Could that argument apply to the underlying hardware, too?

Good question. I never thought about that until you asked. :) It sounds
nice to let udevd serialize the complete event stream, based on
PHYSDEVPATH. I will try it today and come back...

Thanks,
Kay




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevd: serialize device chain event sequence

[Kay Sievers]

[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]

On Thu, Dec 09, 2004 at 09:18:28AM +0100, Kay Sievers wrote:
> On Wed, 2004-12-08 at 19:07 -0800, David Brownell wrote: 
> > On Wednesday 08 December 2004 3:13 pm, Kay Sievers wrote:
> > > Any objections against the serialization of the event sequence of a
> > > chain of devices. Currently udevd delays only events for the same
> > > DEVPATH.

> > But the "add" for the underlying hardware (maybe USB) would
> > not be serialized.
> 
> Right, just the stream of events for the /sys/devices/-device is
> serialized, but in another event sequence.


> > > The main motivation to do this is the program execution of the dev.d/
> > > and hotplug.d/ directory. If we don't wait for the parent event to exit,
> > > we can't be sure that the executed scripts are run in the right order.
> > 
> > Could that argument apply to the underlying hardware, too?
> 
> I will try it today and come back...

I like the idea. We now make sure that the sequence of events for a device
is serialized for every device chain and the class/block devices which have
a "device" link to a physical device are handled after the physical device
is fully populated and notified to userspace. It will only work this way on
kernels later than 2.6.10-rc1 cause it depends on the PHYSDEVPATH value from
the kernel.

Here is an example event sequence with unusual delays are simulated by placing a
script into the /etc/hotplug.d/ directory which sleeps for 3 seconds only for
the events of /block/sda and /devices/pci0000:00/0000:00:1d.1/usb3/3-1.

Thanks,
Kay


The device creation sequences while inserting an USB-stick in the order the
kernel forks the hotplug helper:
  SEQNUM=1034
  DEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1

  SEQNUM=1035
  DEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0

  SEQNUM=1036
  DEVPATH=/class/scsi_host/host3
  PHYSDEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/host3

  SEQNUM=1037
  DEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/host3/target3:0:0/3:0:0:0

  SEQNUM=1038
  DEVPATH=/block/sda
  PHYSDEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/host3/target3:0:0/3:0:0:0

  SEQNUM=1039
  DEVPATH=/block/sda/sda1
  PHYSDEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/host3/target3:0:0/3:0:0:0

  SEQNUM=1040
  DEVPATH=/class/scsi_device/3:0:0:0
  PHYSDEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/host3/target3:0:0/3:0:0:0

  SEQNUM=1041
  DEVPATH=/class/scsi_generic/sg0
  PHYSDEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/host3/target3:0:0/3:0:0:0

serialized sequence visible to hotplug.d/ programs with udevsend as the hotplug helper:
  SEQNUM=1034
    delay seq 1035, cause seq 1034 is still running (serialize /sys/devives/ chain)
  SEQNUM=1035
    delay seq 1036, cause seq 1034 is still running (wait for PHYSDEVPATH)
  SEQNUM=1036
  SEQNUM=1037
  SEQNUM=1040
  SEQNUM=1041
  SEQNUM=1038
    delay seq 1039, cause seq 1038 is still running (serialize /block/* chain)
  SEQNUM=1039


[-- Attachment #2: udevd-serialize-event-sequence-02.patch --]
[-- Type: text/plain, Size: 2689 bytes --]

===== udevd.8 1.10 vs edited =====
--- 1.10/udevd.8	2004-11-28 05:41:14 +01:00
+++ edited/udevd.8	2004-12-09 01:18:36 +01:00
@@ -28,8 +28,6 @@
 .B udevd
 receives the events from
 .B udevsend
-which is called by
-.BR hotplug (8).
 If
 .B udevd
 isn't already running,
===== udevd.c 1.53 vs edited =====
--- 1.53/udevd.c	2004-11-28 05:56:22 +01:00
+++ edited/udevd.c	2004-12-09 09:58:07 +01:00
@@ -151,22 +151,40 @@
 	}
 }
 
-/* returns already running task with devpath */
+/* returns still running task for the same event sequence */
 static struct hotplug_msg *running_with_devpath(struct hotplug_msg *msg)
 {
 	struct hotplug_msg *loop_msg;
+	int i;
+
 	list_for_each_entry(loop_msg, &running_list, list) {
 		if (loop_msg->devpath == NULL || msg->devpath == NULL)
 			continue;
 
-		if (strcmp(loop_msg->devpath, msg->devpath) == 0)
-			return loop_msg;
+		/* is a parent or child device event still running */
+		for (i = 0; i < DEVPATH_SIZE; i++) {
+			if (loop_msg->devpath[i] == '\0' || msg->devpath[i] == '\0')
+				return loop_msg;
+
+			if (loop_msg->devpath[i] != msg->devpath[i])
+				break;
+		}
+
+		/* is the physical device event still running on an add sequence */
+		if (msg->physdevpath && msg->action && strcmp(msg->action, "add") == 0)
+			for (i = 0; i < DEVPATH_SIZE; i++) {
+				if (loop_msg->devpath[i] == '\0' || msg->physdevpath[i] == '\0')
+					return loop_msg;
+
+				if (loop_msg->devpath[i] != msg->physdevpath[i])
+					break;
+			}
 	}
 
 	return NULL;
 }
 
-/* exec queue management routine executes the events and delays events for the same devpath */
+/* exec queue management routine executes the events and serializes events in the same sequence */
 static void exec_queue_manager(void)
 {
 	struct hotplug_msg *loop_msg;
@@ -181,8 +199,8 @@
 			udev_run(loop_msg);
 			dbg("moved seq %llu to running list", loop_msg->seqnum);
 		} else {
-			dbg("delay seq %llu, cause seq %llu already working on '%s'",
-				loop_msg->seqnum, msg->seqnum, msg->devpath);
+			dbg("delay seq %llu (%s), cause seq %llu (%s) is still running",
+			    loop_msg->seqnum, loop_msg->devpath, msg->seqnum, msg->devpath);
 		}
 	}
 }
@@ -314,6 +332,9 @@
 
 		if (strncmp(key, "SEQNUM=", 7) == 0)
 			msg->seqnum = strtoull(&key[7], NULL, 10);
+
+		if (strncmp(key, "PHYSDEVPATH=", 12) == 0)
+			msg->physdevpath = &key[12];
 	}
 	msg->envp[i++] = "MANAGED_EVENT=1";
 	msg->envp[i] = NULL;
===== udevd.h 1.15 vs edited =====
--- 1.15/udevd.h	2004-11-22 22:14:20 +01:00
+++ edited/udevd.h	2004-12-09 09:20:40 +01:00
@@ -47,6 +47,7 @@
 	char *devpath;
 	char *subsystem;
 	unsigned long long seqnum;
+	char *physdevpath;
 	char *envp[HOTPLUG_NUM_ENVP+1];
 	char envbuf[];
 };