Re: module-init-tools/udev and module auto-loading

[Martin Schlemmer]

On Tue, 2004-02-03 at 02:55, Rusty Russell wrote:
> In message <1075748550.6931.10.camel@xxxxxxxxxxxxx> you write:
> > > This does not cover the class of things which are entirely created by
> > > the driver (eg. dummy devices, socket families), so cannot be
> > > "detected".  Many of these (eg. socket families) can be handled by
> > > explicit request_module() in the core and MODULE_ALIAS in the driver.
> > > Some of them cannot at the moment: the first the kernel knows of them
> > > is an attempt to open the device.  Some variant of devfs would solve
> > > this.
> > > 
> > 
> > I guess there will be cries of murder if 'somebody' suggested that if
> > a node in /dev is opened, but not there, the kernel can call
> > 'modprobe -q /dev/foo' to load whatever alias there might have been?
> 
> I think it's an excellent idea, although ideally we would have this
> mechanism in userspace as much as possible.  Anything from some
> special hack to block -ENOENT on directory lookups and notify an fd,
> to some exotic overlay filesystem.
> 

Something like attached.  Besides me not knowing if there is a better
place for it, it have the following issues:

1) Its a bit racy - iow, I get this (but only for urandom):

  request_module: runaway loop modprobe /dev/urandom

I am not sure if its the kthread patches with the one for
kthread use in modules that contributes to this (have not
checked yet).

2) Although it does work (load the right module), it seems to do it
too late.  I have removed the sleep in udev, so it cannot be this.
I do not know if this might be another kthread contribution (if I
understand the module/kthread patches correctly).  It does seem from
the request_module code though that it 'should' wait for modprobe to
complete ...  Maybe add some sort of locking (this be efficient?)?

3) Might be nice to keep track of "/dev/foo" 'aliases' that modprobe
fails for, and not try them again.

4) It only works if the whatever opened's name start with "/dev/", but
I do not think that is an issue, as it should (?) be how most things
access nodes, and should minimise complexity.

I know its very basic, I just thought to get something out of the
door for discussion and to give direction (or what direction not to
take ...).


Thanks,

-- 
Martin Schlemmer
--- 1/fs/namei.c	2004-02-03 14:35:16.000000000 +0200
+++ 2/fs/namei.c	2004-02-03 17:20:21.000000000 +0200
@@ -1240,7 +1240,7 @@ int open_namei(const char * pathname, in
 	int acc_mode, error = 0;
 	struct dentry *dentry;
 	struct dentry *dir;
-	int count = 0;
+	int count = 0, retry = 0;
 
 	acc_mode = ACC_MODE(flag);
 
@@ -1253,13 +1253,14 @@ int open_namei(const char * pathname, in
 	nd->intent.open.flags = flag;
 	nd->intent.open.create_mode = mode;
 
+retry_open:
 	/*
 	 * The simplest case - just a plain lookup.
 	 */
 	if (!(flag & O_CREAT)) {
 		error = path_lookup(pathname, lookup_flags(flag)|LOOKUP_OPEN, nd);
 		if (error)
-			return error;
+			goto error;
 		dentry = nd->dentry;
 		goto ok;
 	}
@@ -1269,7 +1270,7 @@ int open_namei(const char * pathname, in
 	 */
 	error = path_lookup(pathname, LOOKUP_PARENT|LOOKUP_OPEN|LOOKUP_CREATE, nd);
 	if (error)
-		return error;
+		goto error;
 
 	/*
 	 * We have the parent and last component. First of all, check
@@ -1343,8 +1344,22 @@ ok:
 exit_dput:
 	dput(dentry);
 exit:
+	if (error != 0 && strstr(pathname, "/dev/") == pathname && !retry) {
+		if (request_module(pathname) == 0) {
+			retry = 1;
+			goto retry_open;
+		}
+	}
 	path_release(nd);
 	return error;
+error:
+	if (error != 0 && strstr(pathname, "/dev/") == pathname && !retry) {
+		if (request_module(pathname) == 0) {
+			retry = 1;
+			goto retry_open;
+		}
+	}
+	return error;
 
 do_link:
 	error = -ELOOP;

Attachment: signature.asc
Description: This is a digitally signed message part