On Thu, Oct 26, 2000 at 06:21:41PM -0400, Alan Cox wrote:
> > 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.
--
Vojtech Pavlik
SuSE Labs
[email protected] said:
> > 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
On Fri, 27 Oct 2000 14:25:48 +0100,
David Woodhouse <[email protected]> wrote:
>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.
Keith Owens wrote:
>
> 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?
--- linux-2.4.0-test10-pre5/kernel/module.c Tue Oct 24 21:34:13 2000
+++ linux-akpm/kernel/module.c Wed Oct 25 22:11:46 2000
@@ -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;
}
> 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.
> 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
On Sat, 28 Oct 2000 05:40:28 -0400 (EDT),
Alan Cox <[email protected]> wrote:
>> 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.
>> 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
> 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.
> 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
On Sat, 28 Oct 2000 11:02:04 +0100 (BST),
Alan Cox <[email protected]> wrote:
>> 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.