Make agpsupport work with modversions

PATCH Alan Cox 6 participants 10 messages Oct 26, 2000 – Oct 28, 2000 1 message not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alan Cox

author

2000-10-26 22:23 UTC #1 lore raw

Quoted 3 lines

Well, this is usually handled by a third module that takes care of registering/unregistering the existence of the two modules that need to be possible to load/unload separately.

But that module then depends on both of the others unless you keep recompiling it

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Vojtech Pavlik

2000-10-26 22:55 UTC #2 lore raw

In reply to a message not in the archive

Quoted 6 lines · Alan Cox

Quoted 3 lines
Well, this is usually handled by a third module that takes care of registering/unregistering the existence of the two modules that need to be possible to load/unload separately.
But that module then depends on both of the others unless you keep recompiling it

Not really, see for example ns558.c and adi.c plus their third module gameport.c, all in drivers/char/joystick.

Signature · 6 lines

Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

David Woodhouse

2000-10-27 13:29 UTC #3 lore raw

Quoted 5 lines · [email protected] said:

Quoted 2 lines
But that module then depends on both of the others unless you keep recompiling it
Not really, see for example ns558.c and adi.c plus their third module gameport.c, all in drivers/char/joystick.

But in the case where there _aren't_ any functions which could usefully be shared between the modules, you've got a whole extra gratuitous module (What's that, 32KiB on some ARM boxen?) just to hold the registration functions, which aren't needed if you just use get_module_symbol().

Provide generic code for registering such stuff and it might be acceptable. Otherwise, get_module_symbol is better. There's no fundamental flaw with get_module_symbol() - just one or two of the current usages of it.

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Keith Owens

2000-10-28 02:18 UTC #4 lore raw

Quoted 8 lines · David Woodhouse

But in the case where there _aren't_ any functions which could usefully be shared between the modules, you've got a whole extra gratuitous module (What's that, 32KiB on some ARM boxen?) just to hold the registration functions, which aren't needed if you just use get_module_symbol().
Provide generic code for registering such stuff and it might be acceptable. Otherwise, get_module_symbol is better. There's no fundamental flaw with get_module_symbol() - just one or two of the current usages of it.

cc list trimmed. Nobody has come up with a "must have" reason for get_module_symbol and that interface is broken as designed. I will be adding generic inter-object registration code and removing all vestiges of get_module_symbol this weekend. The inter-object registration code will allow two objects to pass data to each other, it will not matter whether the objects are both modules, one module and one built in (in either order) or both built in. When modules are involved there will be full module locking.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Alan Cox

author

2000-10-28 09:41 UTC #5 lore raw

Quoted 2 lines

cc list trimmed. Nobody has come up with a "must have" reason for get_module_symbol and that interface is broken as designed. I will be

Nobody has come up with a 'must break existing sane code' reason either.

Quoted 4 lines

will allow two objects to pass data to each other, it will not matter whether the objects are both modules, one module and one built in (in either order) or both built in. When modules are involved there will be full module locking.

You have no consensus on this. None at all. It is also past the 2.4test point for making this change.

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Keith Owens

2000-10-28 09:55 UTC #6 lore raw

Quoted 4 lines · Alan Cox

Quoted 2 lines
cc list trimmed. Nobody has come up with a "must have" reason for get_module_symbol and that interface is broken as designed. I will be
Nobody has come up with a 'must break existing sane code' reason either.

Existing code is not sane, it does not work with symbol versions. The only code left in the kernel that uses get_module_symbol is agp, drm and mtd, all of which I will be fixing at the same time.

Quoted 7 lines

Quoted 4 lines
will allow two objects to pass data to each other, it will not matter whether the objects are both modules, one module and one built in (in either order) or both built in. When modules are involved there will be full module locking.
You have no consensus on this. None at all. It is also past the 2.4test point for making this change.

Linus wants get_module_symbol removed.
http://www.mail-archive.com/[email protected]/msg08791.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Alan Cox

2000-10-28 10:08 UTC #7 lore raw

Quoted 2 lines

Linus wants get_module_symbol removed.
http://www.mail-archive.com/[email protected]/msg08791.html

Looks to me like Linus asks if some stuff can go away. I don't see a Linus comment on the rest of the discussion about why removing it is bad at all.

And by Linus own rules. Its too late for 2.4 unless you can make Ted agree its a critical fix

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Alan Cox

2000-10-28 10:03 UTC #8 lore raw

In reply to Keith Owens

Quoted 5 lines

of get_module_symbol this weekend. The inter-object registration code will allow two objects to pass data to each other, it will not matter whether the objects are both modules, one module and one built in (in either order) or both built in. When modules are involved there will be full module locking.

Dont forget that one of the objects may not even be present, or may be loaded later.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Keith Owens

2000-10-28 10:08 UTC #9 lore raw

Quoted 8 lines · Alan Cox

Quoted 5 lines
of get_module_symbol this weekend. The inter-object registration code will allow two objects to pass data to each other, it will not matter whether the objects are both modules, one module and one built in (in either order) or both built in. When modules are involved there will be full module locking.
Dont forget that one of the objects may not even be present, or may be loaded later.

How could I forget it? You have defined the heart of the problem, either object might be built into the kernel, might be a module or might not even be there, in any case the load order is undefined. That is why existing code is kludging things by using get_module_symbol(). inter_module_register, unregister, get, put will solve the inter object problem but using a clean interface that works with symbol versions.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Andrew Morton

2000-10-28 07:00 UTC #10 lore raw

In reply to David Woodhouse

Quoted 3 lines · Keith Owens

I will be adding generic inter-object registration code and removing all vestiges of get_module_symbol this weekend.

While you're there, why not eradicate sys_get_kernel_syms()?

Also, I've been sitting on (and using) this sys_init_init_module()
bugfix for two months. The explanation is at http://www.uwsg.iu.edu/hypermail/linux/kernel/0008.3/0379.html

Perhaps now is a good time to merge it?

kernel/module.c +25 -9

@@ -6,6 +6,7 @@
 #include <linux/smp_lock.h>
 #include <asm/pgalloc.h>
 #include <linux/init.h>
+#include <linux/slab.h>
 
 /*
  * Originally by Anonymous (as far as I know...)

@@ -151,7 +152,7 @@
 sys_init_module(const char *name_user, struct module *mod_user)
 {
 	struct module mod_tmp, *mod;
-	char *name, *n_name;
+	char *name, *n_name, *name_tmp = 0;
 	long namelen, n_namelen, i, error;
 	unsigned long mod_user_size;
 	struct module_ref *dep;

@@ -185,6 +186,12 @@
 	/* Hold the current contents while we play with the user's idea
 	   of righteousness.  */
 	mod_tmp = *mod;
+	name_tmp = kmalloc(strlen(mod->name) + 1, GFP_KERNEL);	/* Where's kstrdup()? */
+	if (name_tmp == NULL) {
+		error = -ENOMEM;
+		goto err1;
+	}
+	strcpy(name_tmp, mod->name);
 
 	error = copy_from_user(mod, mod_user, sizeof(struct module));
 	if (error) {

@@ -281,9 +288,10 @@
 	   to make the I and D caches consistent.  */
 	flush_icache_range((unsigned long)mod, (unsigned long)mod + mod->size);
 
-	/* Update module references.  */
 	mod->next = mod_tmp.next;
 	mod->refs = NULL;
+
+	/* Sanity check the module's dependents */
 	for (i = 0, dep = mod->deps; i < mod->ndeps; ++i, ++dep) {
 		struct module *o, *d = dep->dep;

@@ -294,14 +302,21 @@
 			goto err3;
 		}
 
-		for (o = module_list; o != &kernel_module; o = o->next)
-			if (o == d) goto found_dep;
+		/* Scan the current modules for this dependency */
+		for (o = module_list; o != &kernel_module && o != d; o = o->next)
+			;
 
-		printk(KERN_ERR "init_module: found dependency that is "
+		if (o != d) {
+			printk(KERN_ERR "init_module: found dependency that is "
 				"(no longer?) a module.\n");
-		goto err3;
-		
-	found_dep:
+			goto err3;
+		}
+	}
+
+	/* Update module references.  */
+	for (i = 0, dep = mod->deps; i < mod->ndeps; ++i, ++dep) {
+		struct module *d = dep->dep;
+
 		dep->ref = mod;
 		dep->next_ref = d->refs;
 		d->refs = dep;

@@ -335,10 +350,12 @@
 	put_mod_name(n_name);
 err2:
 	*mod = mod_tmp;
+	strcpy((char *)mod->name, name_tmp);	/* We know there is room for this */
 err1:
 	put_mod_name(name);
 err0:
 	unlock_kernel();
+	kfree(name_tmp);
 	return error;
 }
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/