lib.a causing modules not to load

lib.a causing modules not to load

[James Bottomley]

I think this has been mentioned before, but I just ran across it again
recently.  The problem is that if the only reference to a routine in
lib.a is in a module, then it never gets compiled into the kernel, and
the module won't load.

In 2.6.0-test9 this is shown by compiling both ext2 and ext3 as
modules.  Since they're the only things to refer to percpu_counter_mod
which is in lib.a in an SMP system.

The quickest hack I could think of (attached below) was to make ext2
(and ext3 although I didn't code that) incorporate vestigial code into
the discarded init section of the kernel which forces a reference to
percpu_counter_mod and thus makes the kernel pull the required routine
out of lib.a

However, I think the best approach would be, after the kernel has built,
to build the non-included routines of lib.a as individual modules which
would then be pulled in by the module dependency rules.

Unless anyone has a better idea (or has already done something about
this problem), I'll look at doing the latter.

James

Re: lib.a causing modules not to load

[James Bottomley]

On Fri, 2003-11-07 at 10:21, James Bottomley wrote:
[...]

And with the actual patch

James

===== fs/Makefile 1.59 vs edited =====
--- 1.59/fs/Makefile	Thu Aug 14 20:17:09 2003
+++ edited/fs/Makefile	Tue Nov  4 16:33:42 2003
@@ -46,8 +46,10 @@
  
 # Do not add any filesystems before this line
 obj-$(CONFIG_EXT3_FS)		+= ext3/ # Before ext2 so root fs can be ext3
+libobj-$(CONFIG_EXT3_FS)	+= ext3/
 obj-$(CONFIG_JBD)		+= jbd/
 obj-$(CONFIG_EXT2_FS)		+= ext2/
+libobj-$(CONFIG_EXT2_FS)	+= ext2/
 obj-$(CONFIG_CRAMFS)		+= cramfs/
 obj-$(CONFIG_RAMFS)		+= ramfs/
 obj-$(CONFIG_HUGETLBFS)		+= hugetlbfs/
@@ -91,3 +93,6 @@
 obj-$(CONFIG_XFS_FS)		+= xfs/
 obj-$(CONFIG_AFS_FS)		+= afs/
 obj-$(CONFIG_BEFS_FS)		+= befs/
+
+# now pick up the lib resolving refs
+obj-y				+= $(libobj-m)
\ No newline at end of file
===== fs/ext2/Makefile 1.10 vs edited =====
--- 1.10/fs/ext2/Makefile	Sat Jul 19 16:53:59 2003
+++ edited/fs/ext2/Makefile	Tue Nov  4 16:32:35 2003
@@ -3,6 +3,10 @@
 #
 
 obj-$(CONFIG_EXT2_FS) += ext2.o
+libobj-$(CONFIG_EXT2_FS) := librefs.o
+
+# if we're a module, add our lib requirements to the kernel
+obj-y	+= $(libobj-m)
 
 ext2-y := balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
 	  ioctl.o namei.o super.o symlink.o
--- /dev/null	2003-11-02 22:37:48.000000000 -0600
+++ edited/fs/ext2/librefs.c	2003-11-05 11:42:37.000000000 -0600
@@ -0,0 +1,7 @@
+#include <linux/init.h>
+#include <linux/percpu_counter.h>
+
+__init __attribute__((unused)) static void dummy(void)
+{
+	percpu_counter_mod(NULL, 0);
+}

Re: lib.a causing modules not to load

[Andrew Morton]

James Bottomley <James.Bottomley@steeleye.com> wrote:
>
> I think this has been mentioned before, but I just ran across it again
>  recently.  The problem is that if the only reference to a routine in
>  lib.a is in a module, then it never gets compiled into the kernel, and
>  the module won't load.
> 
>  In 2.6.0-test9 this is shown by compiling both ext2 and ext3 as
>  modules.  Since they're the only things to refer to percpu_counter_mod
>  which is in lib.a in an SMP system.

How about we just link that function into the kernel and be done with it? 
We'll waste a few bytes on SMP machines which have neither ext2 nor ext3
linked-in or loaded as modules, but that doesn't sound very important...

(We don't have a kernel/random-support-stuff.c, but we have
mm/random-support-stuff.c which for some reason is called mm/swap.c, so
I put it there).



diff -puN -L lib/percpu_counter.c lib/percpu_counter.c~percpu-counter-linkage-fix /dev/null
--- 25/lib/percpu_counter.c
+++ /dev/null	2002-08-30 16:31:37.000000000 -0700
@@ -1,21 +0,0 @@
-#include <linux/module.h>
-#include <linux/percpu_counter.h>
-#include <linux/sched.h>
-
-void percpu_counter_mod(struct percpu_counter *fbc, long amount)
-{
-	int cpu = get_cpu();
-	long count = fbc->counters[cpu].count;
-
-	count += amount;
-	if (count >= FBC_BATCH || count <= -FBC_BATCH) {
-		spin_lock(&fbc->lock);
-		fbc->count += count;
-		spin_unlock(&fbc->lock);
-		count = 0;
-	}
-	fbc->counters[cpu].count = count;
-	put_cpu();
-}
-
-EXPORT_SYMBOL(percpu_counter_mod);
diff -puN lib/Makefile~percpu-counter-linkage-fix lib/Makefile
--- 25/lib/Makefile~percpu-counter-linkage-fix	2003-11-07 20:26:09.000000000 -0800
+++ 25-akpm/lib/Makefile	2003-11-07 20:26:24.000000000 -0800
@@ -9,7 +9,6 @@ lib-y := errno.o ctype.o string.o vsprin
 
 lib-$(CONFIG_RWSEM_GENERIC_SPINLOCK) += rwsem-spinlock.o
 lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
-lib-$(CONFIG_SMP) += percpu_counter.o
 
 ifneq ($(CONFIG_HAVE_DEC_LOCK),y) 
   lib-y += dec_and_lock.o
diff -puN mm/swap.c~percpu-counter-linkage-fix mm/swap.c
--- 25/mm/swap.c~percpu-counter-linkage-fix	2003-11-07 20:26:09.000000000 -0800
+++ 25-akpm/mm/swap.c	2003-11-07 20:28:10.000000000 -0800
@@ -14,6 +14,7 @@
  */
 
 #include <linux/mm.h>
+#include <linux/sched.h>
 #include <linux/kernel_stat.h>
 #include <linux/swap.h>
 #include <linux/mman.h>
@@ -23,6 +24,8 @@
 #include <linux/module.h>
 #include <linux/mm_inline.h>
 #include <linux/buffer_head.h>	/* for try_to_release_page() */
+#include <linux/module.h>
+#include <linux/percpu_counter.h>
 #include <linux/percpu.h>
 
 /* How many pages do we try to swap or page in/out together? */
@@ -380,6 +383,24 @@ void vm_acct_memory(long pages)
 EXPORT_SYMBOL(vm_acct_memory);
 #endif
 
+#ifdef CONFIG_SMP
+void percpu_counter_mod(struct percpu_counter *fbc, long amount)
+{
+	int cpu = get_cpu();
+	long count = fbc->counters[cpu].count;
+
+	count += amount;
+	if (count >= FBC_BATCH || count <= -FBC_BATCH) {
+		spin_lock(&fbc->lock);
+		fbc->count += count;
+		spin_unlock(&fbc->lock);
+		count = 0;
+	}
+	fbc->counters[cpu].count = count;
+	put_cpu();
+}
+EXPORT_SYMBOL(percpu_counter_mod);
+#endif
 
 /*
  * Perform any setup for the swap system

_

Re: lib.a causing modules not to load

[Christoph Hellwig]

On Fri, Nov 07, 2003 at 08:34:19PM -0800, Andrew Morton wrote:
> James Bottomley <James.Bottomley@steeleye.com> wrote:
> >
> > I think this has been mentioned before, but I just ran across it again
> >  recently.  The problem is that if the only reference to a routine in
> >  lib.a is in a module, then it never gets compiled into the kernel, and
> >  the module won't load.
> > 
> >  In 2.6.0-test9 this is shown by compiling both ext2 and ext3 as
> >  modules.  Since they're the only things to refer to percpu_counter_mod
> >  which is in lib.a in an SMP system.
> 
> How about we just link that function into the kernel and be done with it? 
> We'll waste a few bytes on SMP machines which have neither ext2 nor ext3
> linked-in or loaded as modules, but that doesn't sound very important...
> 
> (We don't have a kernel/random-support-stuff.c, but we have
> mm/random-support-stuff.c which for some reason is called mm/swap.c, so
> I put it there).

Well, this solves the problem for this particular case, but not other
stuff in lib for other situations.  We should just stop building lib
as archive and conditionalize building bigger and rarely used stuff in
there using Kconfig symbols.

Re: lib.a causing modules not to load

[James Bottomley]

On Sat, 2003-11-08 at 02:51, Christoph Hellwig wrote:
> On Fri, Nov 07, 2003 at 08:34:19PM -0800, Andrew Morton wrote:
> > How about we just link that function into the kernel and be done with it? 
> > We'll waste a few bytes on SMP machines which have neither ext2 nor ext3
> > linked-in or loaded as modules, but that doesn't sound very important...
> > 
> > (We don't have a kernel/random-support-stuff.c, but we have
> > mm/random-support-stuff.c which for some reason is called mm/swap.c, so
> > I put it there).
> 
> Well, this solves the problem for this particular case, but not other
> stuff in lib for other situations.

I agree...there's much more in lib than just percpu_counter_mod.

> We should just stop building lib
> as archive and conditionalize building bigger and rarely used stuff in
> there using Kconfig symbols.

Actually, I do think lib serves a purpose for shared routines that are
used in disparate places throughout the kernel.

Why code these as Kconfig symbol dependencies when the library mechanism
does exactly what we want except for this one all symbols are in modules
case.  As a counter example: kobject is in lib...Imagine exactly how
many Kconfig conditions we'd have to introduce to conditionalise that...

I really think the correct way forwards is to fix the one failing case;
I was just asking if anyone had already thought about it.

James

Re: lib.a causing modules not to load

[Rusty Russell]

In message <1068304571.2048.5.camel@mulgrave> you write:
> On Sat, 2003-11-08 at 02:51, Christoph Hellwig wrote:
> > On Fri, Nov 07, 2003 at 08:34:19PM -0800, Andrew Morton wrote:
> > > How about we just link that function into the kernel and be done with it?
 
> > > We'll waste a few bytes on SMP machines which have neither ext2 nor ext3
> > > linked-in or loaded as modules, but that doesn't sound very important...
> > > 
> > > (We don't have a kernel/random-support-stuff.c, but we have
> > > mm/random-support-stuff.c which for some reason is called mm/swap.c, so
> > > I put it there).
> > 
> > Well, this solves the problem for this particular case, but not other
> > stuff in lib for other situations.
> 
> I agree...there's much more in lib than just percpu_counter_mod.

I think lib.a should be linked as is if !CONFIG_MODULES, and done as a
..o if CONFIG_MODULES.  Other alternatives are possible, but make it
tricky if someone adds a module later which wants something in lib.a.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: lib.a causing modules not to load

[James Bottomley]

On Sun, 2003-11-16 at 20:47, Rusty Russell wrote:
> I think lib.a should be linked as is if !CONFIG_MODULES, and done as a
> ..o if CONFIG_MODULES.  Other alternatives are possible, but make it
> tricky if someone adds a module later which wants something in lib.a.

I tried this and it is getting to be a whole nasty mess can of worms:

You can't link the lib objects all in, because they can be overridden by
the arch dependent lib.a (we rely on link order to permit this).  For
instance on x86, dump_stack and bust_spinlocks give duplicate symbols.

If we have to add processing logic to work out what symbols are in the
arch lib.a, we might as well go the whole hog and try to work out what
is missing from the kernel and compile that alone as a module.

James

Re: lib.a causing modules not to load

[Rusty Russell]

In message <1069172719.1835.30.camel@mulgrave> you write:
> On Sun, 2003-11-16 at 20:47, Rusty Russell wrote:
> > I think lib.a should be linked as is if !CONFIG_MODULES, and done as a
> > ..o if CONFIG_MODULES.  Other alternatives are possible, but make it
> > tricky if someone adds a module later which wants something in lib.a.
> 
> I tried this and it is getting to be a whole nasty mess can of worms:
> 
> You can't link the lib objects all in, because they can be overridden by
> the arch dependent lib.a (we rely on link order to permit this).  For
> instance on x86, dump_stack and bust_spinlocks give duplicate symbols.

Ewww.... Weak symbols?

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.