Re: [PATCH] Make agpsupport work with modversions

2000-10-28 Thread Andrew Morton

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;
 }
-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-28 Thread Alan Cox

 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

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-28 Thread Keith Owens

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/linux-kernel@vger.kernel.org/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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-28 Thread Alan Cox

 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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-28 Thread Alan Cox

 Linus wants get_module_symbol removed.
 http://www.mail-archive.com/linux-kernel@vger.kernel.org/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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-28 Thread Keith Owens

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.

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-27 Thread David Woodhouse


[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


-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-27 Thread Keith Owens

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.

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-26 Thread Alan Cox

 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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-26 Thread Vojtech Pavlik

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
-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-19 Thread Vojtech Pavlik

On Wed, Oct 18, 2000 at 10:14:01PM +0100, Alan Cox wrote:
  The only other users are 8390.h and a couple of mtd things. I don't see
  why this stuff cannot be handled in userspace with /etc/modules.conf ...
  
  should get_module_symbol() die ?
 
 You need it to dynamically bind to another module if its loaded and still be
 loadable if that module/facility is not present. Its dynamic linking for kernel
 modules 

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.

-- 
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-19 Thread David Woodhouse


[EMAIL PROTECTED] said:
  You need it to dynamically bind to another module if its loaded and
 still be loadable if that module/facility is not present. Its dynamic
 linking for kernel modules  

However, in order for get_module_symbol() to be safe, it needs to 
increase the use count of the module in which it finds the symbol, and the 
corresponding put_module_symbol() function has to be provided. This was 
done in 2.4 a while ago. Patch for 2.2 attached.

Index: kernel/ksyms.c
===
RCS file: /inst/cvs/linux/kernel/ksyms.c,v
retrieving revision 1.13
diff -u -r1.13 ksyms.c
--- kernel/ksyms.c  2000/10/16 09:43:22 1.13
+++ kernel/ksyms.c  2000/10/19 08:18:08
@@ -81,6 +81,7 @@
 
 #ifdef CONFIG_MODULES
 EXPORT_SYMBOL(get_module_symbol);
+EXPORT_SYMBOL(put_module_symbol);
 #endif
 EXPORT_SYMBOL(get_options);
 
Index: kernel/module.c
===
RCS file: /inst/cvs/linux/kernel/module.c,v
retrieving revision 1.2
diff -u -r1.2 module.c
--- kernel/module.c 2000/06/07 10:00:30 1.2
+++ kernel/module.c 2000/10/19 08:18:08
@@ -952,7 +952,9 @@
  * Gets the address for a symbol in the given module.  If modname is
  * NULL, it looks for the name in any registered symbol table.  If the
  * modname is an empty string, it looks for the symbol in kernel exported
- * symbol tables.
+ * symbol tables. Increase the usage count of the module in which the
+ * symbol was found - it's the only way we can guarantee that it's still
+ * there by the time our caller actually uses it.
  */
 unsigned long
 get_module_symbol(char *modname, char *symname)
@@ -969,12 +971,29 @@
i  0; --i, ++sym) {
 
if (strcmp(sym-name, symname) == 0) {
+   __MOD_INC_USE_COUNT(mp);
return sym-value;
}
}
}
}
return 0;
+}
+
+/* Decrease the use count of the module containing a symbol with the 
+ * address passed.
+ */
+void put_module_symbol(unsigned long addr)
+{
+   struct module *mp;
+
+   for (mp = module_list; mp; mp = mp-next) {
+   if (addr = (unsigned long)mp 
+   addr  (unsigned long)mp + mp-size) {
+   __MOD_DEC_USE_COUNT(mp);
+   return;
+   }
+   }
 }
 
 #else  /* CONFIG_MODULES */
Index: include/linux/module.h
===
RCS file: /inst/cvs/linux/include/linux/module.h,v
retrieving revision 1.3
diff -u -r1.3 module.h
--- include/linux/module.h  2000/09/11 08:34:17 1.3
+++ include/linux/module.h  2000/10/19 08:19:48
@@ -144,8 +144,10 @@
 /* Find a symbol exported by the kernel or another module */
 #ifndef CONFIG_MODULES
 static inline unsigned long get_module_symbol(char *A, char *B) { return 0; };
+static inline void put_module_symbol(unsigned long A) { };
 #else
 extern unsigned long get_module_symbol(char *, char *);
+extern void put_module_symbol(unsigned long);
 #endif
 #if defined(MODULE)  !defined(__GENKSYMS__)
 


--
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-19 Thread Vojtech Pavlik

On Wed, Oct 18, 2000 at 02:25:41PM -0400, Rik Faith wrote:
 Just to clarify -- my use of get_module_symbol has nothing to do with
 load order.  It has to do with allowing a drm module to work with or
 without the agpgart module loaded.
 
 If there's some other way to do this, I'll be happy to move to that.
 I'd like to preserve the ability to have one driver which works with
 each chipset family and not have to have separate drivers for PCI and
 AGP cards (or, rather, to have fewer instances of the drivers -- they
 already depend on SMP and MODVERSIONS, and that's a lot of overhead
 already if you just want to play Q3A :).  I'd also like a dual-head
 system to be able to load the same drm module and just have it work
 for both the agp and the pci cards (this isn't supported yet, in case
 anyone is wondering).

Just do it the way most other subsystems do it - have a
drm_[un]register_driver() functions, which are exported by the DRM
generic module and have the card-specific modules call that. This way
the DRM generic module will know what drivers are loaded.

For an example see the PCI subsystem, USB, Input, chardevices, TTY
layer, whatever ...

-- 
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread John Levon

On Tue, 17 Oct 2000, Linus Torvalds wrote:

 There's something else wrong in the config to make this be needed at all.
 You need to figure out what the real problem is, and what is causing the
 AGP symbols to not get version information. Probably a file is missing
 from the "export-objs" list..


There was no export-objs list at all ;)

I have only compile-tested the patch below with 2.4.0test10pre3 and
2.2.18pre16 (some fuzz on apply). Hope it's right, I can't test if it
fixes the MODVERSIONS+in kernel agp+in kernel drm case. I tested kernel
and module cases.

I don't actually see the point of get_module_symbol at all - what is a
valid use for it ? it is ugly to have a CONFIG_MODULES requirement for a
monolithic kernel, and it has an inaccurate name (since kernel_module
isn't a module ;)

john

diff -Nuar linux/drivers/char/agp/Makefile new/drivers/char/agp/Makefile
--- linux/drivers/char/agp/Makefile Tue Jan  4 17:41:45 2000
+++ new/drivers/char/agp/Makefile   Wed Oct 18 13:55:59 2000
@@ -3,20 +3,28 @@
 # space ioctl interface to use agp memory.  It also adds a kernel interface
 # that other drivers could use to manipulate agp memory.
 
-O_TARGET   := agp.o
+O_TARGET := agp.o
 
-ifeq ($(CONFIG_AGP),y)
-  O_OBJS += agpgart_fe.o
-  OX_OBJS += agpgart_be.o
-else
-  ifeq ($(CONFIG_AGP), m)
-MI_OBJS += agpgart_fe.o
-MIX_OBJS += agpgart_be.o
-M_OBJS += agpgart.o
-  endif
-endif
+agpgart-objs := agpgart_be.o agpgart_fe.o
+
+export-objs := agpgart_be.o
+
+obj-$(CONFIG_AGP) += agpgart.o $(agpgart-objs)
+
+# Take module names out of obj-y and int-m
+ 
+obj-y   := $(filter-out agpgart.o, $(obj-y))
+int-m   := $(filter-out agpgart.o, $(obj-m))
+ 
+# Translate to Rules.make lists.
+ 
+O_OBJS  := $(filter-out $(export-objs), $(obj-y))
+OX_OBJS := $(filter $(export-objs), $(obj-y))
+M_OBJS  := $(sort $(filter agpgart.o, $(obj-m)))
+MI_OBJS := $(sort $(filter-out $(export-objs), $(int-m)))
+MIX_OBJS:= $(sort $(filter $(export-objs), $(int-m)))
 
 include $(TOPDIR)/Rules.make
 
-agpgart.o: agpgart_be.o agpgart_fe.o
-   $(LD) $(LD_RFLAG) -r -o $@ agpgart_be.o agpgart_fe.o
+agpgart.o: $(agpgart-objs)
+   $(LD) -r -o $@ $(agpgart-objs)
diff -Nuar linux/drivers/char/agp/agpgart_be.c new/drivers/char/agp/agpgart_be.c
--- linux/drivers/char/agp/agpgart_be.c Mon Aug 21 16:08:12 2000
+++ new/drivers/char/agp/agpgart_be.c   Wed Oct 18 13:29:24 2000
@@ -421,7 +421,7 @@
 
 /* Generic Agp routines - Start */
 
-static void agp_generic_agp_enable(u32 mode)
+static void __attribute__((unused)) agp_generic_agp_enable(u32 mode)
 {
struct pci_dev *device = NULL;
u32 command, scratch, cap_id;
@@ -712,7 +712,7 @@
return 0;
 }
 
-static int agp_generic_insert_memory(agp_memory * mem,
+static int __attribute__((unused)) agp_generic_insert_memory(agp_memory * mem,
 off_t pg_start, int type)
 {
int i, j, num_entries;
@@ -770,7 +770,7 @@
return 0;
 }
 
-static int agp_generic_remove_memory(agp_memory * mem, off_t pg_start,
+static int __attribute__((unused)) agp_generic_remove_memory(agp_memory * mem, off_t 
+pg_start,
 int type)
 {
int i;
@@ -788,12 +788,12 @@
return 0;
 }
 
-static agp_memory *agp_generic_alloc_by_type(size_t page_count, int type)
+static agp_memory * __attribute__((unused)) agp_generic_alloc_by_type(size_t 
+page_count, int type)
 {
return NULL;
 }
 
-static void agp_generic_free_by_type(agp_memory * curr)
+static void __attribute__((unused)) agp_generic_free_by_type(agp_memory * curr)
 {
if (curr-memory != NULL) {
vfree(curr-memory);
diff -Nuar linux/drivers/char/drm/Config.in new/drivers/char/drm/Config.in
--- linux/drivers/char/drm/Config.inTue Aug  8 17:27:33 2000
+++ new/drivers/char/drm/Config.in  Wed Oct 18 13:08:01 2000
@@ -10,6 +10,6 @@
 tristate '  3dfx Banshee/Voodoo3+' CONFIG_DRM_TDFX
 tristate '  3dlabs GMX 2000' CONFIG_DRM_GAMMA
 tristate '  ATI Rage 128' CONFIG_DRM_R128
-dep_tristate '  Intel I810' CONFIG_DRM_I810 $CONFIG_AGP
-dep_tristate '  Matrox g200/g400' CONFIG_DRM_MGA $CONFIG_AGP
+dep_tristate '  Intel I810' CONFIG_DRM_I810 $CONFIG_AGP $CONFIG_MODULES
+dep_tristate '  Matrox g200/g400' CONFIG_DRM_MGA $CONFIG_AGP $CONFIG_MODULES
 fi

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Linus Torvalds



On Wed, 18 Oct 2000, John Levon wrote:
 
 I have only compile-tested the patch below with 2.4.0test10pre3 and
 2.2.18pre16 (some fuzz on apply). Hope it's right, I can't test if it
 fixes the MODVERSIONS+in kernel agp+in kernel drm case. I tested kernel
 and module cases.

It looks better.

However, the fact that you need that dependency on CONFIG_MODULES _still_
shows that something is wrong. That dependency should not be there, and
the drm code should be fixed. Why does it care about CONFIG_MODULES at
all? It should not, and it _must_ not do that.

I have no idea what the get_module_symbol() code in question is trying to
do, but this should be _fixed_ and not just worked around. That's a bug.

Linus

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread John Levon

On Wed, 18 Oct 2000, Linus Torvalds wrote:

 It looks better.
 
 However, the fact that you need that dependency on CONFIG_MODULES _still_
 shows that something is wrong. That dependency should not be there, and
 the drm code should be fixed. Why does it care about CONFIG_MODULES at
 all? It should not, and it _must_ not do that.
 
 I have no idea what the get_module_symbol() code in question is trying to
 do, but this should be _fixed_ and not just worked around. That's a bug.
 
   Linus

For some reason this code is there to allow agp to be a module when drm is
built into the kernel. I don't know why - the maintainers should comment
on this one ...

It is because of get_module_symbol() in the first place (of course a
no-op without CONFIG_MODULES). I can't see a reason for this routine to
exist *at all* really. 

The only other users are 8390.h and a couple of mtd things. I don't see
why this stuff cannot be handled in userspace with /etc/modules.conf ...

should get_module_symbol() die ?

john 

-- 
"Mathemeticians stand on each other's shoulders while computer scientists
 stand on each other's toes."
- Richard Hamming


-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Christoph Hellwig

In article [EMAIL PROTECTED] you wrote:

 However, the fact that you need that dependency on CONFIG_MODULES _still_
 shows that something is wrong. That dependency should not be there, and
 the drm code should be fixed. Why does it care about CONFIG_MODULES at
 all? It should not, and it _must_ not do that.

Because get_module_symbol() needs CONFIG_MODULES. 
IMHO we should simply get rid of get_module_symbol in drm.

 I have no idea what the get_module_symbol() code in question is trying to
 do, but this should be _fixed_ and not just worked around. That's a bug.

It gets the symbol of a function, that's name is passed as char * to
get_module_symbol().

Christoph

-- 
Always remember that you are unique.  Just like everyone else.
-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread David Woodhouse


[EMAIL PROTECTED] said:
  The only other users are 8390.h and a couple of mtd things. I don't
 see why this stuff cannot be handled in userspace with /etc/
 modules.conf ...

Patch please :)

Either you'll see, or I'll owe you a beer. I'm happy either way. 

 should get_module_symbol() die ? 

Not IMHO. Certainly not until the existing uses of it are replaced with
viable alternatives.

If drm is using it wrongly, fix drm, don't take get_module_symbol away. 

--
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Linus Torvalds



On Wed, 18 Oct 2000, Christoph Hellwig wrote:

 In article [EMAIL PROTECTED] you 
wrote:
 
  I have no idea what the get_module_symbol() code in question is trying to
  do, but this should be _fixed_ and not just worked around. That's a bug.
 
 It gets the symbol of a function, that's name is passed as char * to
 get_module_symbol().

Oh, I know what get_module_symbol() does - I just don't understand why drm
would want to use it. There are basically no good uses for it, as far as
I'm concerned.

I assume it's something about allowing modules to be loaded in the wrong
order. Some other drivers did hooks like this in times past. The right
solution was then, and is almost certainly now, to just remove the code in
question and force the correct load order.

Rik? Can this code just go away?

Linus

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Rik Faith

On Wed 18 Oct 2000 09:43:43 -0700,
   Linus Torvalds [EMAIL PROTECTED] wrote:
 
 
 On Wed, 18 Oct 2000, John Levon wrote:
  
  I have only compile-tested the patch below with 2.4.0test10pre3 and
  2.2.18pre16 (some fuzz on apply). Hope it's right, I can't test if it
  fixes the MODVERSIONS+in kernel agp+in kernel drm case. I tested kernel
  and module cases.
 
 It looks better.

We apparently had never changed the Makefile over to the new format.
The dependency on CONFIG_MODULES is essentially orthogonal to this
problem.

 However, the fact that you need that dependency on CONFIG_MODULES _still_
 shows that something is wrong. That dependency should not be there, and
 the drm code should be fixed. Why does it care about CONFIG_MODULES at
 all? It should not, and it _must_ not do that.
 
 I have no idea what the get_module_symbol() code in question is trying to
 do, but this should be _fixed_ and not just worked around. That's a bug.

The get_module_symbol() call will get the agp symbols if agpgart is
available (either as a loaded module, or compiled into the kernel).
Some DRM drivers optionally make use of agpgart, so it's useful for
the DRM driver to look for agpgart support at run time even if it
isn't available (e.g., the R128 driver may use AGP on an AGP system,
but not on a PCI system; or you might have the same type of chip on an
AGP card and on a PCI card, and you'd like to be able to load the same
drm module for both cards).

There are several cases that the configuration can have:

CONFIG_AGP  CONFIG_MODULES
n   n or y no agpsupport.c, so no problem

m or y  y  agpsupport.c uses get_module_symbol,
   and the drm runs with or without
   agpgart support (which may or may
   not be a module that may or may not
   be loaded).  [This is the reason
   we're using get_module_symbol -- I
   didn't realize it wasn't available
   if CONFIG_MODULES was 'n'.]

y   n  This is the broken case, which can
   be fixed if agpsupport.c uses the
   #ifdef below -- this case ensures
   that the agpgart module is built
   into the monolithic kernel and we
   can safely hardcode the function
   names at compile time.

#ifdef CONFIG_MODULES
for (fill = drm_agp_fill[0]; fill-name; fill++) {
char *n  = (char *)fill-name;
*fill-f = (drm_agp_func_u)get_module_symbol(NULL, n);
DRM_DEBUG("%s resolves to 0x%08lx\n", n, (*fill-f).address);
if (!(*fill-f).address) agp_available = 0;
}
#else
drm_agp_fill[0].f = agp_free_memory;
drm_agp_fill[1].f = agp_allocate_memory;
drm_agp_fill[2].f = agp_bind_memory;
drm_agp_fill[3].f = agp_unbind_memory;
drm_agp_fill[4].f = agp_enable;
drm_agp_fill[5].f = agp_backend_acquire;
drm_agp_fill[6].f = agp_backend_release;
drm_agp_fill[7].f = agp_copy_info;
#endif



[Note that the other way to fix this would be to export
get_module_symbol all the time, and have it just search the available
symbol space if CONFIG_MODULES is 'n'.]
-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread John Levon

On Wed, 18 Oct 2000, Rik Faith wrote:

 [Note that the other way to fix this would be to export
 get_module_symbol all the time, and have it just search the available
 symbol space if CONFIG_MODULES is 'n'.]

and 

s/_module//;

it is mis-named already ...

john

-- 
"Mathemeticians stand on each other's shoulders while computer scientists
 stand on each other's toes."
- Richard Hamming

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Rik Faith

On Wed 18 Oct 2000 10:49:24 -0700,
   Linus Torvalds [EMAIL PROTECTED] wrote:
 
 
 On Wed, 18 Oct 2000, Christoph Hellwig wrote:
 
  In article [EMAIL PROTECTED] you 
wrote:
  
   I have no idea what the get_module_symbol() code in question is trying to
   do, but this should be _fixed_ and not just worked around. That's a bug.
  
  It gets the symbol of a function, that's name is passed as char * to
  get_module_symbol().
 
 Oh, I know what get_module_symbol() does - I just don't understand why drm
 would want to use it. There are basically no good uses for it, as far as
 I'm concerned.
 
 I assume it's something about allowing modules to be loaded in the wrong
 order. Some other drivers did hooks like this in times past. The right
 solution was then, and is almost certainly now, to just remove the code in
 question and force the correct load order.

Just to clarify -- my use of get_module_symbol has nothing to do with
load order.  It has to do with allowing a drm module to work with or
without the agpgart module loaded.

If there's some other way to do this, I'll be happy to move to that.
I'd like to preserve the ability to have one driver which works with
each chipset family and not have to have separate drivers for PCI and
AGP cards (or, rather, to have fewer instances of the drivers -- they
already depend on SMP and MODVERSIONS, and that's a lot of overhead
already if you just want to play Q3A :).  I'd also like a dual-head
system to be able to load the same drm module and just have it work
for both the agp and the pci cards (this isn't supported yet, in case
anyone is wondering).

 Rik? Can this code just go away?

See above and my mail from a few minutes ago.  I'd like to keep it
(with the naming changes John Levon proposed, even if we don't make it
available when CONFIG_MODULES is 'n').
-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread David Woodhouse



[EMAIL PROTECTED] said:
 #ifdef CONFIG_MODULES

/* use get_module_symbol() */

 #else

/* reference agp_* directly */

 #endif


Don't you need to deal with the !CONFIG_AGP case correctly? 

#ifdef CONFIG_MODULES
/* blah */
#elif CONFIG_AGP
/* blah */
agp_available = 1;
#else
agp_available = 0;
#endif


[EMAIL PROTECTED] said:
 [Note that the other way to fix this would be to export
 get_module_symbol all the time, and have it just search the available
 symbol space if CONFIG_MODULES is 'n'.]

There is no available symbol space if CONFIG_MODULES is 'n'. 

--
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Rik Faith

On Wed 18 Oct 2000 19:23:54 +0100,
   David Woodhouse [EMAIL PROTECTED] wrote:
 Don't you need to deal with the !CONFIG_AGP case correctly?

This should already be dealt with in the Makefile -- if !CONFIG_AGP,
then the file that we've been talking about (agpsupport.c) isn't
compiled.

(So, yes, you can still customize a drm module for your specific
kernel.  But I'm arguing for the ability to build a generic drm module
that will determine agpgart presence at run time and use it if needed.)

 [EMAIL PROTECTED] said:
  [Note that the other way to fix this would be to export
  get_module_symbol all the time, and have it just search the available
  symbol space if CONFIG_MODULES is 'n'.]
 
 There is no available symbol space if CONFIG_MODULES is 'n'. 

Oh :(

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread David Woodhouse


[EMAIL PROTECTED] said:
  (So, yes, you can still customize a drm module for your specific
 kernel.  But I'm arguing for the ability to build a generic drm module
 that will determine agpgart presence at run time and use it if
 needed.) 

Definitely worthwhile. Just don't make anything in the drm module depend on 
CONFIG_AGP_MODULE. 

[EMAIL PROTECTED] said:
  There is no available symbol space if CONFIG_MODULES is 'n'. 
 Oh :(

What would be the point? If it's in the kernel image, you can just link to 
it directly. If it's not, then it never can be, so you might as well not 
bother.

--
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Alan Cox

 Nice and clean.  WEAK_EXTERN does some magic to create a NULL pointer
 at link time or load time if the symbol is not resolved.

It also has to do the rest of the magic to handle module load/unload in 
parallel but that can be done as per the current code

 Linus, do you want a patch for this?

2.5 surely

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Keith Owens

On Wed, 18 Oct 2000 20:44:19 -0400 (EDT), 
Alan Cox [EMAIL PROTECTED] wrote:
Keith Owens wrote
 Nice and clean.  WEAK_EXTERN does some magic to create a NULL pointer
 at link time or load time if the symbol is not resolved.

It also has to do the rest of the magic to handle module load/unload in 
parallel but that can be done as per the current code

modprobe would attempt to satisfy weak external references as if they
were normal references, including all the module dependency chains and
reference counts.  If the reference cannot be satisfied, it is set to
zero instead of causing an error.  No changes to load/unload.

 Linus, do you want a patch for this?

2.5 surely

Normally I would agree but the existing code is going to cause problems
all the way through 2.4.  This only affects drivers/net/8390.h (all
8390 cards?) drivers/char/drm/agpsupport.c, drivers/mtd/cfi_probe.c,
drivers/mtd/docprobe.c.  As currently coded, some of those simply will
not work 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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Alan Cox

 modprobe would attempt to satisfy weak external references as if they
 were normal references, including all the module dependency chains and
 reference counts.  If the reference cannot be satisfied, it is set to
 zero instead of causing an error.  No changes to load/unload.

I dont believe modprobe can do this race free in userspace

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-18 Thread Keith Owens

On Thu, 19 Oct 2000 01:56:38 +0100 (BST), 
Alan Cox [EMAIL PROTECTED] wrote:
Keith Owens wrote
 modprobe would attempt to satisfy weak external references as if they
 were normal references, including all the module dependency chains and
 reference counts.  If the reference cannot be satisfied, it is set to
 zero instead of causing an error.  No changes to load/unload.

I dont believe modprobe can do this race free in userspace

Module dependency checking in userspace has always been racy.  A is
being loaded and needs a symbol from B.  B was loaded at the start of
insmod so the symbol was resolved.  By the time A is actually loaded, B
has been removed, userspace race.  This is checked for in
sys_init_module()

printk(KERN_ERR "init_module: found dependency that is "
"(no longer?) a module.\n");

Making some of the external references weak makes no difference.
Either they are resolved to a module by insmod and checked by
sys_init_module() or insmod replaces the reference with NULL.

-
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/



Re: [PATCH] Make agpsupport work with modversions

2000-10-17 Thread Linus Torvalds



On Tue, 17 Oct 2000, John Levon wrote:
 
 The patch below allows agpsupport to find the agp functions
 when modversions is set and both AGP and DRM are compiled into the kernel,
 and adds the dependency on CONFIG_MODULES explicitly.

There's something else wrong in the config to make this be needed at all.
You need to figure out what the real problem is, and what is causing the
AGP symbols to not get version information. Probably a file is missing
from the "export-objs" list..

Linus

-
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/