[LWN Logo]
[Timeline]
From: Keith Owens <kaos@ocs.com.au>
To: miles@megapathdsl.net
Subject: Re: (Linus, please respond!) Re: Roadmap to restoring working usb module autoloading? 
Date: Sun, 21 Jan 2001 13:36:47 +1100

On Sat, 20 Jan 2001 17:57:30 -0800, 
Miles Lane <miles@megapathdsl.net> wrote:
>"Adam J. Richter" wrote:
>> 
>>         I am concerned that the situation with usb module autoloading
>> seems to be deadlocked and so I want to propose a roadmap for
>> getting to the point where usb module autoloading works when a
>> user installs the latest versions of everything and does not
>> make any additional modifications.
>
>I agree that we are deadlocked.  However, I think we should continue
>to try to get Linus to accept Kieth's patch, at least until he gives
>us a solid reason for rejecting the patch.

For the record.  The addition of match_flags in struct usb_device_id
highlighted an ABI problem between the kernel and depmod.  It proved
that relying on the table size to detect format changes was a bad
idea(TM), adding a field will not always change the table size.  The
table version numbers added in my patch (appended) are _required_ for
depmod to reliably detect that a format has changed.

Checking kernel version will not work.  That method requires users to
upgrade to a new modutils that knows about the changes from one kernel
version to another.  Users do not upgrade when told to, relying on
kernel version guarantees that some users will try to read new table
formats with old modutils and get random errors.  The ABI exporter must
define the interface version, not the ABI consumer.

Index: 0.1/include/linux/usb.h
--- 0.1/include/linux/usb.h Fri, 05 Jan 2001 13:42:29 +1100 kaos (linux-2.4/Z/38_usb.h 1.1 644)
+++ 0.1(w)/include/linux/usb.h Sun, 07 Jan 2001 22:36:31 +1100 kaos (linux-2.4/Z/38_usb.h 1.1 644)
@@ -344,12 +344,18 @@ struct usb_device;
 #define USB_INTERFACE_INFO(cl,sc,pr) \
 	match_flags: USB_DEVICE_ID_MATCH_INT_INFO, bInterfaceClass: (cl), bInterfaceSubClass: (sc), bInterfaceProtocol: (pr)
 
-struct usb_device_id {
-	/* This bitmask is used to determine which of the following fields
-	 * are to be used for matching.
-	 */
-	__u16		match_flags;
+/* match_flags added in 2.4.0 but at the start which messed up depmod.
+ * match_flags moved to before driver_info in 2.4.1 by KAO, you also need
+ * modutils 2.4.1.  USB modules cannot be supported in kernel 2.4.0,
+ * insufficient data to detect which table format is being used.
+ *
+ * Do NOT change this table format without checking with the modutils
+ * maintainer.  This is an ABI visible structure.
+ */
+
+#define usb_device_id_ver	2	/* Version 2 table */
 
+struct usb_device_id {
 	/*
 	 * vendor/product codes are checked, if vendor is nonzero
 	 * Range is for device revision (bcdDevice), inclusive;
@@ -374,6 +380,11 @@ struct usb_device_id {
 	__u8		bInterfaceClass;
 	__u8		bInterfaceSubClass;
 	__u8		bInterfaceProtocol;
+
+	/* This bitmask is used to determine which of the preceding fields
+	 * are to be used for matching.
+	 */
+	__u16		match_flags;	/* New in version 2 */
 
 	/*
 	 * for driver's use; not involved in driver matching.
Index: 0.1/include/linux/isapnp.h
--- 0.1/include/linux/isapnp.h Fri, 05 Jan 2001 13:42:29 +1100 kaos (linux-2.4/b/b/11_isapnp.h 1.1 644)
+++ 0.1(w)/include/linux/isapnp.h Sun, 07 Jan 2001 22:36:40 +1100 kaos (linux-2.4/b/b/11_isapnp.h 1.1 644)
@@ -142,6 +142,16 @@ struct isapnp_resources {
 #define ISAPNP_CARD_TABLE(name) \
 		MODULE_GENERIC_TABLE(isapnp_card, name)
 
+/* Do NOT change the format of struct isapnp_card_id, struct isapnp_device_id or
+ * the value of ISAPNP_CARD_DEVS without checking with the modutils maintainer.
+ * These are ABI visible structures and defines.
+ *
+ * isapnp_device_id_ver is a single version number for the combination of
+ * struct isapnp_card_id and struct isapnp_device_id.
+ */
+
+#define isapnp_device_id_ver	1	/* Version 1 tables */
+
 struct isapnp_card_id {
 	unsigned long driver_data;	/* data private to the driver */
 	unsigned short card_vendor, card_device;
Index: 0.1/include/linux/module.h
--- 0.1/include/linux/module.h Fri, 05 Jan 2001 13:42:29 +1100 kaos (linux-2.4/c/b/46_module.h 1.1 644)
+++ 0.1(w)/include/linux/module.h Sun, 07 Jan 2001 22:07:49 +1100 kaos (linux-2.4/c/b/46_module.h 1.1 644)
@@ -242,19 +242,17 @@ __attribute__((section(".modinfo"))) =		
  * isapnp - struct isapnp_device_id - List of ISA PnP ids supported by this module
  * usb - struct usb_device_id - List of USB ids supported by this module
  */
-#define MODULE_GENERIC_TABLE(gtype,name)	\
-static const unsigned long __module_##gtype##_size \
-  __attribute__ ((unused)) = sizeof(struct gtype##_id); \
-static const struct gtype##_id * __module_##gtype##_table \
-  __attribute__ ((unused)) = name
-#define MODULE_DEVICE_TABLE(type,name)		\
-  MODULE_GENERIC_TABLE(type##_device,name)
-/* not put to .modinfo section to avoid section type conflicts */
 
-/* The attributes of a section are set the first time the section is
-   seen; we want .modinfo to not be allocated.  */
+#define MODULE_GENERIC_TABLE(gtype,name)			\
+static const unsigned long __module_##gtype##_size		\
+  __attribute__ ((unused)) = sizeof(struct gtype##_id);		\
+static const unsigned long __module_##gtype##_ver		\
+  __attribute__ ((unused)) = gtype##_id_ver;			\
+static const struct gtype##_id * __module_##gtype##_table	\
+  __attribute__ ((unused)) = name
 
-__asm__(".section .modinfo\n\t.previous");
+#define MODULE_DEVICE_TABLE(type,name)				\
+  MODULE_GENERIC_TABLE(type##_device,name)
 
 /* Define the module variable, and usage macros.  */
 extern struct module __this_module;
Index: 0.1/include/linux/pci.h
--- 0.1/include/linux/pci.h Fri, 05 Jan 2001 13:42:29 +1100 kaos (linux-2.4/f/b/12_pci.h 1.1 644)
+++ 0.1(w)/include/linux/pci.h Sun, 07 Jan 2001 22:36:09 +1100 kaos (linux-2.4/f/b/12_pci.h 1.1 644)
@@ -439,6 +439,12 @@ struct pbus_set_ranges_data
 	unsigned long mem_start, mem_end;
 };
 
+/* Do NOT change this table format without checking with the modutils
+ * maintainer.  This is an ABI visible structure.
+ */
+
+#define pci_device_id_ver	1	/* Version 1 table */
+
 struct pci_device_id {
 	unsigned int vendor, device;		/* Vendor and device ID or PCI_ANY_ID */
 	unsigned int subvendor, subdevice;	/* Subsystem ID's or PCI_ANY_ID */
Index: 0.1/Documentation/Changes
--- 0.1/Documentation/Changes Fri, 05 Jan 2001 13:42:29 +1100 kaos (linux-2.4/Z/c/26_Changes 1.1 644)
+++ 0.4(w)/Documentation/Changes Tue, 09 Jan 2001 02:43:44 +1100 kaos (linux-2.4/Z/c/26_Changes 1.1 644)
@@ -52,7 +52,7 @@ o  Gnu C                  2.91.66       
 o  Gnu make               3.77                    # make --version
 o  binutils               2.9.1.0.25              # ld -v
 o  util-linux             2.10o                   # fdformat --version
-o  modutils               2.4.0                   # insmod -V
+o  modutils               2.4.1                   # insmod -V
 o  e2fsprogs              1.19                    # tune2fs --version
 o  pcmcia-cs              3.1.21                  # cardmgr -V
 o  PPP                    2.4.0                   # pppd --version


_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel