Re: udevd: serialize device chain event sequence

[Kay Sievers <kay.sievers@vrfy.org>]

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