2004-09-20 17:30:19

by James Bottomley

[permalink] [raw]
Subject: [RFC] put symbolic links between drivers and modules in the sysfs tree

This functionality is essential for us to work out which drivers are
supplied by which modules. We use this in turn to work out which
modules are necessary to find the root device (and hence what
initrd/initramfs needs to insert).

If you look at debian at the moment, it uses a huge mapping table on
/proc/scsi/* to do this. If we implement the sysfs feature, we can
simply go from /sys/block/<device> to the actual device to the driver
and then to the module with no need of any fixed tables.

The code is a first cut and introduces two new module APIs:
module_add_driver() and module_remove_driver(). We need this because
the generic device model driver has no current knowledge of modules. We
could enhance it to have a struct module * in struct device_driver, but
once we have the sysfs links which this patch provides, there didn't
seem to be a compelling reason to add it to struct device_driver.

Comments?

James

===== drivers/base/bus.c 1.62 vs edited =====
--- 1.62/drivers/base/bus.c 2004-07-07 19:55:49 -05:00
+++ edited/drivers/base/bus.c 2004-09-20 11:09:52 -05:00
@@ -553,6 +553,7 @@
pr_debug("bus %s: remove driver %s\n", drv->bus->name, drv->name);
driver_detach(drv);
up_write(&drv->bus->subsys.rwsem);
+ module_remove_driver(drv);
kobject_unregister(&drv->kobj);
put_bus(drv->bus);
}
===== drivers/scsi/hosts.c 1.101 vs edited =====
--- 1.101/drivers/scsi/hosts.c 2004-09-09 16:16:26 -05:00
+++ edited/drivers/scsi/hosts.c 2004-09-20 10:49:08 -05:00
@@ -130,6 +130,11 @@
goto out_del_classdev;

scsi_proc_host_add(shost);
+ if (shost->hostt->module && shost->shost_gendev.parent &&
+ shost->shost_gendev.parent->driver)
+ module_add_driver(shost->hostt->module,
+ shost->shost_gendev.parent->driver);
+
return error;

out_del_classdev:
===== include/linux/module.h 1.79 vs edited =====
--- 1.79/include/linux/module.h 2004-06-27 02:19:28 -05:00
+++ edited/include/linux/module.h 2004-09-20 11:06:13 -05:00
@@ -578,6 +578,9 @@

#define __MODULE_STRING(x) __stringify(x)

+/* forward reference for the module_add/remove_driver API */
+struct device_driver;
+
/* Use symbol_get and symbol_put instead. You'll thank me. */
#define HAVE_INTER_MODULE
extern void inter_module_register(const char *, struct module *, const void *);
@@ -585,5 +588,7 @@
extern const void *inter_module_get(const char *);
extern const void *inter_module_get_request(const char *, const char *);
extern void inter_module_put(const char *);
+extern void module_add_driver(struct module *, struct device_driver *);
+extern void module_remove_driver(struct device_driver *);

#endif /* _LINUX_MODULE_H */
===== kernel/module.c 1.120 vs edited =====
--- 1.120/kernel/module.c 2004-09-08 01:33:04 -05:00
+++ edited/kernel/module.c 2004-09-20 11:10:47 -05:00
@@ -34,6 +34,7 @@
#include <linux/vermagic.h>
#include <linux/notifier.h>
#include <linux/stop_machine.h>
+#include <linux/device.h>
#include <asm/uaccess.h>
#include <asm/semaphore.h>
#include <asm/cacheflush.h>
@@ -2139,6 +2140,30 @@
printk(" %s", mod->name);
printk("\n");
}
+
+void module_add_driver(struct module *mod, struct device_driver *drv)
+{
+ if (!mod || !drv)
+ return;
+ if (!mod->mkobj)
+ return;
+
+ /* Note: Perhaps we should store this link in the
+ * device_driver structure as well (i.e. have a struct module *
+ * element). */
+
+ /* Don't check return code; this call is idempotent */
+ sysfs_create_link(&drv->kobj, &mod->mkobj->kobj, "module");
+}
+EXPORT_SYMBOL(module_add_driver);
+
+void module_remove_driver(struct device_driver *drv)
+{
+ if (!drv)
+ return;
+ sysfs_remove_link(&drv->kobj, "module");
+}
+EXPORT_SYMBOL(module_remove_driver);

#ifdef CONFIG_MODVERSIONS
/* Generate the signature for struct module here, too, for modversions. */




2004-09-22 23:07:35

by Greg KH

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Wed, Sep 22, 2004 at 04:04:23PM -0700, Greg KH wrote:
> I'll post my usb core change after this, to show you how USB can
> be hooked up to it.

And here's the 3 line patch that I added to the usb core to hook up both
the usb and usb-serial drivers to support the modules symlinks.

I'll go mess with the pci core now, but as there is no "struct module *"
in the pci driver structure, it will take a bit of auditing to get them
all hooked up properly.

thanks,

greg k-h

------


USB: add support for symlink from usb and usb-serial driver to its module in sysfs

Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c 2004-09-22 15:56:44 -07:00
+++ b/drivers/usb/core/usb.c 2004-09-22 15:56:44 -07:00
@@ -76,6 +76,7 @@
}

static struct device_driver usb_generic_driver = {
+ .owner = THIS_MODULE,
.name = "usb",
.bus = &usb_bus_type,
.probe = generic_probe,
@@ -159,6 +160,7 @@
new_driver->driver.bus = &usb_bus_type;
new_driver->driver.probe = usb_probe_interface;
new_driver->driver.remove = usb_unbind_interface;
+ new_driver->driver.owner = new_driver->owner;

usb_lock_all_devices();
retval = driver_register(&new_driver->driver);
diff -Nru a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
--- a/drivers/usb/serial/bus.c 2004-09-22 15:56:44 -07:00
+++ b/drivers/usb/serial/bus.c 2004-09-22 15:56:44 -07:00
@@ -120,6 +120,7 @@
device->driver.bus = &usb_serial_bus_type;
device->driver.probe = usb_serial_device_probe;
device->driver.remove = usb_serial_device_remove;
+ device->driver.owner = device->owner;

retval = driver_register(&device->driver);

2004-09-22 23:08:10

by Greg KH

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Mon, Sep 20, 2004 at 01:29:44PM -0400, James Bottomley wrote:
> This functionality is essential for us to work out which drivers are
> supplied by which modules. We use this in turn to work out which
> modules are necessary to find the root device (and hence what
> initrd/initramfs needs to insert).
>
> If you look at debian at the moment, it uses a huge mapping table on
> /proc/scsi/* to do this. If we implement the sysfs feature, we can
> simply go from /sys/block/<device> to the actual device to the driver
> and then to the module with no need of any fixed tables.
>
> The code is a first cut and introduces two new module APIs:
> module_add_driver() and module_remove_driver(). We need this because
> the generic device model driver has no current knowledge of modules. We
> could enhance it to have a struct module * in struct device_driver, but
> once we have the sysfs links which this patch provides, there didn't
> seem to be a compelling reason to add it to struct device_driver.
>
> Comments?

I like it. But I do think that this should be moved into the driver
core, so I added a "struct module *" to "struct device_driver". Here's
the patch that I just applied to my trees, based on yours. It is only
the core changes (and yes, I did fix up the location of the functions in
the module.h file and fixed the bug when modules were not configured
in.) I'll post my usb core change after this, to show you how USB can
be hooked up to it. I'll let you figure out what the best way to handle
the scsi code would be.

thanks again for the patch.

greg k-h

---------


Put symbolic links between drivers and modules in the sysfs tree

This functionality is essential for us to work out which drivers are
supplied by which modules. We use this in turn to work out which
modules are necessary to find the root device (and hence what
initrd/initramfs needs to insert).

If you look at debian at the moment, it uses a huge mapping table on
/proc/scsi/* to do this. If we implement the sysfs feature, we can
simply go from /sys/block/<device> to the actual device to the driver
and then to the module with no need of any fixed tables.

Signed-off-by: James Bottomley <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c 2004-09-22 15:56:32 -07:00
+++ b/drivers/base/bus.c 2004-09-22 15:56:32 -07:00
@@ -529,6 +529,7 @@
down_write(&bus->subsys.rwsem);
driver_attach(drv);
up_write(&bus->subsys.rwsem);
+ module_add_driver(drv->owner, drv);

driver_add_attrs(bus, drv);
}
@@ -553,6 +554,7 @@
pr_debug("bus %s: remove driver %s\n", drv->bus->name, drv->name);
driver_detach(drv);
up_write(&drv->bus->subsys.rwsem);
+ module_remove_driver(drv);
kobject_unregister(&drv->kobj);
put_bus(drv->bus);
}
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h 2004-09-22 15:56:32 -07:00
+++ b/include/linux/device.h 2004-09-22 15:56:32 -07:00
@@ -106,6 +106,8 @@
struct kobject kobj;
struct list_head devices;

+ struct module * owner;
+
int (*probe) (struct device * dev);
int (*remove) (struct device * dev);
void (*shutdown) (struct device * dev);
diff -Nru a/include/linux/module.h b/include/linux/module.h
--- a/include/linux/module.h 2004-09-22 15:56:32 -07:00
+++ b/include/linux/module.h 2004-09-22 15:56:32 -07:00
@@ -451,6 +451,11 @@
int unregister_module_notifier(struct notifier_block * nb);

extern void print_modules(void);
+
+struct device_driver;
+void module_add_driver(struct module *, struct device_driver *);
+void module_remove_driver(struct device_driver *);
+
#else /* !CONFIG_MODULES... */
#define EXPORT_SYMBOL(sym)
#define EXPORT_SYMBOL_GPL(sym)
@@ -540,6 +545,15 @@
static inline void print_modules(void)
{
}
+
+static inline void module_add_driver(struct module *, struct device_driver *)
+{
+}
+
+static inline void module_remove_driver(struct device_driver *)
+{
+}
+
#endif /* CONFIG_MODULES */

#define symbol_request(x) try_then_request_module(symbol_get(x), "symbol:" #x)
diff -Nru a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c 2004-09-22 15:56:32 -07:00
+++ b/kernel/module.c 2004-09-22 15:56:32 -07:00
@@ -34,6 +34,7 @@
#include <linux/vermagic.h>
#include <linux/notifier.h>
#include <linux/stop_machine.h>
+#include <linux/device.h>
#include <asm/uaccess.h>
#include <asm/semaphore.h>
#include <asm/cacheflush.h>
@@ -2187,6 +2188,26 @@
printk(" %s", mod->name);
printk("\n");
}
+
+void module_add_driver(struct module *mod, struct device_driver *drv)
+{
+ if (!mod || !drv)
+ return;
+ if (!mod->mkobj)
+ return;
+
+ /* Don't check return code; this call is idempotent */
+ sysfs_create_link(&drv->kobj, &mod->mkobj->kobj, "module");
+}
+EXPORT_SYMBOL(module_add_driver);
+
+void module_remove_driver(struct device_driver *drv)
+{
+ if (!drv)
+ return;
+ sysfs_remove_link(&drv->kobj, "module");
+}
+EXPORT_SYMBOL(module_remove_driver);

#ifdef CONFIG_MODVERSIONS
/* Generate the signature for struct module here, too, for modversions. */

2004-09-22 23:41:49

by Greg KH

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Wed, Sep 22, 2004 at 04:06:50PM -0700, Greg KH wrote:
> On Wed, Sep 22, 2004 at 04:04:23PM -0700, Greg KH wrote:
> > I'll post my usb core change after this, to show you how USB can
> > be hooked up to it.
>
> And here's the 3 line patch that I added to the usb core to hook up both
> the usb and usb-serial drivers to support the modules symlinks.
>
> I'll go mess with the pci core now, but as there is no "struct module *"
> in the pci driver structure, it will take a bit of auditing to get them
> all hooked up properly.

Here's that patch, if anyone cares...

thanks,

greg k-h

------


PCI: add "struct module *" to struct pci_driver to show symlink in sysfs for pci drivers.

Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff -Nru a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
--- a/drivers/pci/pci-driver.c 2004-09-22 16:24:57 -07:00
+++ b/drivers/pci/pci-driver.c 2004-09-22 16:24:57 -07:00
@@ -417,6 +417,7 @@
drv->driver.bus = &pci_bus_type;
drv->driver.probe = pci_device_probe;
drv->driver.remove = pci_device_remove;
+ drv->driver.owner = drv->owner;
drv->driver.kobj.ktype = &pci_driver_kobj_type;
pci_init_dynids(&drv->dynids);

diff -Nru a/include/linux/pci.h b/include/linux/pci.h
--- a/include/linux/pci.h 2004-09-22 16:24:57 -07:00
+++ b/include/linux/pci.h 2004-09-22 16:24:57 -07:00
@@ -632,9 +632,11 @@
unsigned int use_driver_data:1; /* pci_driver->driver_data is used */
};

+struct module;
struct pci_driver {
struct list_head node;
char *name;
+ struct module *owner;
const struct pci_device_id *id_table; /* must be non-NULL for probe to be called */
int (*probe) (struct pci_dev *dev, const struct pci_device_id *id); /* New device inserted */
void (*remove) (struct pci_dev *dev); /* Device removed (NULL if not a hot-plug capable driver) */

2004-09-25 07:38:23

by Al Viro

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Mon, Sep 20, 2004 at 01:29:44PM -0400, James Bottomley wrote:
> This functionality is essential for us to work out which drivers are
> supplied by which modules. We use this in turn to work out which
> modules are necessary to find the root device (and hence what
> initrd/initramfs needs to insert).

So what will your userland code do when you run it on a system with
non-modular kernel currently running?

IOW, that's a fundamentally broken interface - you really want the same
information regardless of modular vs. built-in.

2004-09-25 08:05:54

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

[email protected] wrote:
> On Mon, Sep 20, 2004 at 01:29:44PM -0400, James Bottomley wrote:
>> This functionality is essential for us to work out which drivers are
>> supplied by which modules. We use this in turn to work out which
>> modules are necessary to find the root device (and hence what
>> initrd/initramfs needs to insert).
>
> So what will your userland code do when you run it on a system with
> non-modular kernel currently running?

Totally agreed. If we didn't care about built-in drivers, then we might
as well do cat /proc/modules.

BTW, I'm very glad that this is being worked on and that table in Debian's
mkinitrd can finally die.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2004-09-25 08:22:14

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Sat, 2004-09-25 at 10:05, Herbert Xu wrote:

> BTW, I'm very glad that this is being worked on and that table in Debian's
> mkinitrd can finally die.

btw does that mkinitrd already use
readlink /sys/block/sda/device/block/device

(which gives
../../devices/pci0000:00/0000:00:06.0/0000:03:0b.0/host1/1:0:0:0
as output)

the pci path that gives can easily be matched to modules.pcimap to find
the information in case of a PCI device, so at least the table in your
mkinitrd doesn't need to contain PCI devices.....


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2004-09-25 13:15:28

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Sat, 2004-09-25 at 03:38, [email protected]
wrote:
> On Mon, Sep 20, 2004 at 01:29:44PM -0400, James Bottomley wrote:
> > This functionality is essential for us to work out which drivers are
> > supplied by which modules. We use this in turn to work out which
> > modules are necessary to find the root device (and hence what
> > initrd/initramfs needs to insert).
>
> So what will your userland code do when you run it on a system with
> non-modular kernel currently running?

Not put a module in the initial ramdisk, since it would be unnecessary.
The only information the patch seeks to add is the linkage between
driver and module. So you can work back from sysfs to know which
devices have which modules

> IOW, that's a fundamentally broken interface - you really want the same
> information regardless of modular vs. built-in.

Not really. It you argue this about module *parameters*, I might agree,
but not about what driver goes with what module...if the driver is build
in, then the answer is a simple "none" and the link doesn't exist.

James


2004-09-25 13:17:59

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Sat, 2004-09-25 at 04:21, Arjan van de Ven wrote:
> btw does that mkinitrd already use
> readlink /sys/block/sda/device/block/device
>
> (which gives
> ../../devices/pci0000:00/0000:00:06.0/0000:03:0b.0/host1/1:0:0:0
> as output)
>
> the pci path that gives can easily be matched to modules.pcimap to find
> the information in case of a PCI device, so at least the table in your
> mkinitrd doesn't need to contain PCI devices.....

So tell me what happens when my root device is on MCA (which is one of
my machines) or is a parisc internal device (which is another)...

Putting the modules link in relieves mkinitrd from having to understand
anything about the bus types.

James


2004-09-25 16:48:13

by Greg KH

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Sat, Sep 25, 2004 at 08:38:19AM +0100, [email protected] wrote:
> On Mon, Sep 20, 2004 at 01:29:44PM -0400, James Bottomley wrote:
> > This functionality is essential for us to work out which drivers are
> > supplied by which modules. We use this in turn to work out which
> > modules are necessary to find the root device (and hence what
> > initrd/initramfs needs to insert).
>
> So what will your userland code do when you run it on a system with
> non-modular kernel currently running?
>
> IOW, that's a fundamentally broken interface - you really want the same
> information regardless of modular vs. built-in.

I agree, and Rusty has some pending patches that provide that
information for all drivers built into the system. When they are
merged, this symlink will be created for those also (with a bit of
tweaking, but it will happen.)

thanks,

greg k-h

2004-09-26 10:37:49

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

James Bottomley <[email protected]> wrote:
>
>> So what will your userland code do when you run it on a system with
>> non-modular kernel currently running?
>
> Not put a module in the initial ramdisk, since it would be unnecessary.
> The only information the patch seeks to add is the linkage between
> driver and module. So you can work back from sysfs to know which
> devices have which modules

You're assuming that the kernel before/after the reboot have the same
configuration. This is false in general.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2004-09-26 13:10:14

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC] put symbolic links between drivers and modules in the sysfs tree

On Sun, 2004-09-26 at 06:37, Herbert Xu wrote:
> James Bottomley <[email protected]> wrote:
> >
> >> So what will your userland code do when you run it on a system with
> >> non-modular kernel currently running?
> >
> > Not put a module in the initial ramdisk, since it would be unnecessary.
> > The only information the patch seeks to add is the linkage between
> > driver and module. So you can work back from sysfs to know which
> > devices have which modules
>
> You're assuming that the kernel before/after the reboot have the same
> configuration. This is false in general.

No I'm not. For an initrd/initramfs the only assumption would be that
the boot device's driver is compiled in or modular. If this isn't true,
the system won't boot anyway.

James