2.5.31: modules don't work at all

2.5.31: modules don't work at all

[Michel Eyckmans (MCE)]

After upgrading from 2.5.30 to 2.5.31, nothing related to modules 
works for me. Insmod, rmmod, you name it. They all cause errors
along the line of: "QM_SYMBOLS: Bad Address". Any suggestions?

This is with the very latest modutils (2.4.19). These work fine 
with 2.5.30.

MCE

Re: 2.5.31: modules don't work at all

[Flanigan, Ryan]

>>>>> "Michel" == Michel Eyckmans <(MCE)" <mce@pi.be>> writes:

    Michel> After upgrading from 2.5.30 to 2.5.31, nothing related to
    Michel> modules works for me. Insmod, rmmod, you name it. They all
    Michel> cause errors along the line of: "QM_SYMBOLS: Bad
    Michel> Address". Any suggestions?
 
is CONFIG_PREEMPT set?

Re: 2.5.31: modules don't work at all

[Andrew Rodland]

On Sun, 11 Aug 2002 14:41:36 +0200
"Michel Eyckmans (MCE)" <mce@pi.be> wrote:

> 
> After upgrading from 2.5.30 to 2.5.31, nothing related to modules 
> works for me. Insmod, rmmod, you name it. They all cause errors
> along the line of: "QM_SYMBOLS: Bad Address". Any suggestions?
> 
> This is with the very latest modutils (2.4.19). These work fine 
> with 2.5.30.

Ditto here.
Ryan: Yes, CONFIG_PREEMPT is set.

--hobbs

Re: 2.5.31: modules don't work at all

[Flanigan, Ryan]

>>>>> "Andrew" == Andrew Rodland <arodland@noln.com> writes:

    Andrew> On Sun, 11 Aug 2002 14:41:36 +0200 "Michel Eyckmans (MCE)"
    Andrew> <mce@pi.be> wrote:

    >> After upgrading from 2.5.30 to 2.5.31, nothing related to
    >> modules works for me. Insmod, rmmod, you name it. They all
    >> cause errors along the line of: "QM_SYMBOLS: Bad Address". Any
    >> suggestions?

    Andrew> Ditto here.  Ryan: Yes, CONFIG_PREEMPT is set.

try "unsetting" it.  (same problem on the 2.5.31 kernels where i had it set)

Re: 2.5.31: modules don't work at all

[Adam J. Richter]

Ryan Flanigan writes:
>>>>>> "Andrew" == Andrew Rodland <arodland@noln.com> writes:
>
>    Andrew> On Sun, 11 Aug 2002 14:41:36 +0200 "Michel Eyckmans (MCE)"
>    Andrew> <mce@pi.be> wrote:
>
>    >> After upgrading from 2.5.30 to 2.5.31, nothing related to
>    >> modules works for me. Insmod, rmmod, you name it. They all
>    >> cause errors along the line of: "QM_SYMBOLS: Bad Address". Any
>    >> suggestions?
>
>    Andrew> Ditto here.  Ryan: Yes, CONFIG_PREEMPT is set.
>
>try "unsetting" it.  (same problem on the 2.5.31 kernels where i had it set)

	I am also experiencing modules not working with CONFIG_PREEMPT
set, and deactivating CONFIG_PREEMPT works around the problem for me too.

	Ryan, thanks for suggesting that, as it would have taken me a
long time to narrow it down that far!

	It would help avoid duplication of effort if you could indicate
how along you are with this problem.  If you or someone else has nailed
the problem and is preparing a patch, then there is no point in anyone
else trying to duplicate that debugging effort.  On the other hand,
if you just noticed CONFIG_PREEMPT was the difference between your
configuration and that of someone else who was running 2.5.31 successfully
and are not actively debugging the problem, then I'll try to poke at it
some more.

	I already know that the error that trips insmod occurs at
in modules.c, line 831, when qm_symbols gets an error from copy_to_user():

        for (; i < mod->nsyms ; ++i, ++s, vals += 2) {
                len = strlen(s->name)+1;
                if (len > bufsize)
                        goto calc_space_needed;

here------>     if (copy_to_user(strings, s->name, len)
                    || __put_user(s->value, vals+0)
                    || __put_user(space, vals+1))
                        return -EFAULT;

                strings += len;
                bufsize -= len;
                space += len;
        }

	The values of strings and s->name are similar in 2.5.30+preempt
(works) and 2.5.31+preempt (does not work).  strings is 0x08______, and
s->name is 0xc0______.

Adam J. Richter     __     ______________   575 Oroville Road
adam@yggdrasil.com     \ /                  Milpitas, California 95035
+1 408 309-6081         | g g d r a s i l   United States of America
                         "Free Software For The Rest Of Us."

Re: 2.5.31: modules don't work at all

[Skip Ford]

Adam J. Richter wrote:
>
> 	I am also experiencing modules not working with CONFIG_PREEMPT
> set, and deactivating CONFIG_PREEMPT works around the problem for me too.
> 
[snip]
> 
> 	I already know that the error that trips insmod occurs at
> in modules.c, line 831, when qm_symbols gets an error from copy_to_user():
> 
>         for (; i < mod->nsyms ; ++i, ++s, vals += 2) {
>                 len = strlen(s->name)+1;
>                 if (len > bufsize)
>                         goto calc_space_needed;
> 
> here------>     if (copy_to_user(strings, s->name, len)
>                     || __put_user(s->value, vals+0)
>                     || __put_user(space, vals+1))
>                         return -EFAULT;
> 
>                 strings += len;
>                 bufsize -= len;
>                 space += len;
>         }
> 
> 	The values of strings and s->name are similar in 2.5.30+preempt
> (works) and 2.5.31+preempt (does not work).  strings is 0x08______, and
> s->name is 0xc0______.

If I back out this change to arch/i386/mm/fault.c then modules
successfully load.  I have no idea if backing it out causes other
problems though.

diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
--- a/arch/i386/mm/fault.c	Sat Aug 10 18:42:20 2002
+++ b/arch/i386/mm/fault.c	Sat Aug 10 18:42:20 2002
@@ -181,10 +181,10 @@
 	info.si_code = SEGV_MAPERR;
 
 	/*
-	 * If we're in an interrupt or have no user
-	 * context, we must not take the fault..
+	 * If we're in an interrupt, have no user context or are running in an
+	 * atomic region then we must not take the fault..
 	 */
-	if (in_interrupt() || !mm)
+	if (preempt_count() || !mm)
 		goto no_context;
 
 	down_read(&mm->mmap_sem);


-- 
Skip

Re: 2.5.31: modules don't work at all

[Andrew Morton]

Skip Ford wrote:
> 
> ...
> >       I already know that the error that trips insmod occurs at
> > in modules.c, line 831, when qm_symbols gets an error from copy_to_user():
> >
> >         for (; i < mod->nsyms ; ++i, ++s, vals += 2) {
> >                 len = strlen(s->name)+1;
> >                 if (len > bufsize)
> >                         goto calc_space_needed;
> >
> > here------>     if (copy_to_user(strings, s->name, len)
> >                     || __put_user(s->value, vals+0)
> >                     || __put_user(space, vals+1))
> >                         return -EFAULT;
> >
> >                 strings += len;
> >                 bufsize -= len;
> >                 space += len;
> >         }
> >
> >       The values of strings and s->name are similar in 2.5.30+preempt
> > (works) and 2.5.31+preempt (does not work).  strings is 0x08______, and
> > s->name is 0xc0______.
> 
> If I back out this change to arch/i386/mm/fault.c then modules
> successfully load.  I have no idea if backing it out causes other
> problems though.
> 
> diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
> --- a/arch/i386/mm/fault.c      Sat Aug 10 18:42:20 2002
> +++ b/arch/i386/mm/fault.c      Sat Aug 10 18:42:20 2002
> @@ -181,10 +181,10 @@
>         info.si_code = SEGV_MAPERR;
> 
>         /*
> -        * If we're in an interrupt or have no user
> -        * context, we must not take the fault..
> +        * If we're in an interrupt, have no user context or are running in an
> +        * atomic region then we must not take the fault..
>          */
> -       if (in_interrupt() || !mm)
> +       if (preempt_count() || !mm)
>                 goto no_context;
> 
>         down_read(&mm->mmap_sem);
> 

Yes, that's the problem.   qm_symbols() is performing copy_to_user()
inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
But only if preempt is selected.  The copy_to_user() doesn't work.

There's nothing illegal about copy_to_user() inside lock_kernel().

Linus, we can back out the preempt_count() test in there and
perform the atomic copy_*_user via a current->flags bit, or
we can do something else?

Re: 2.5.31: modules don't work at all

[Linus Torvalds]

On Sun, 11 Aug 2002, Andrew Morton wrote:
> 
> Yes, that's the problem.   qm_symbols() is performing copy_to_user()
> inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
> But only if preempt is selected.  The copy_to_user() doesn't work.
> 
> There's nothing illegal about copy_to_user() inside lock_kernel().
> 
> Linus, we can back out the preempt_count() test in there and
> perform the atomic copy_*_user via a current->flags bit, or
> we can do something else?

Since I'm actually hoping that the kernel lock goes away some day, and I 
don't want to pollute the stuff that I hope will _not_ go away, I'd prefer 
a slightly different approach, namely make kernel_lock() special from a 
preempt_count() angle.

In particular, we already "sort" the preemtion count bits according to
just how atomic we are, and lock_kernel is certainly "less atomic" than a
spinlock. So the logical thing to do (I think) is to just make that more
explicit, and make lock_kernel use the low bit of preempt_count, and make
regular spinlocks do a "+= 2" instead of a "+= 1".

That way preempt_count() gives you a much better picture of what the state
of this process is (the name "preempt_count" really gives the wrong notion
these days, since it's really much more generic and is already used for
things that have little to do with preemption any more)

Robert, mind looking into this?

			Linus

Re: 2.5.31: modules don't work at all

[Andrew Morton]

Linus Torvalds wrote:
> 
> On Sun, 11 Aug 2002, Andrew Morton wrote:
> >
> > Yes, that's the problem.   qm_symbols() is performing copy_to_user()
> > inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
> > But only if preempt is selected.  The copy_to_user() doesn't work.
> >
> > There's nothing illegal about copy_to_user() inside lock_kernel().
> >
> > Linus, we can back out the preempt_count() test in there and
> > perform the atomic copy_*_user via a current->flags bit, or
> > we can do something else?
> 
> Since I'm actually hoping that the kernel lock goes away some day, and I
> don't want to pollute the stuff that I hope will _not_ go away, I'd prefer
> a slightly different approach, namely make kernel_lock() special from a
> preempt_count() angle.
> 
> In particular, we already "sort" the preemtion count bits according to
> just how atomic we are, and lock_kernel is certainly "less atomic" than a
> spinlock. So the logical thing to do (I think) is to just make that more
> explicit, and make lock_kernel use the low bit of preempt_count, and make
> regular spinlocks do a "+= 2" instead of a "+= 1".

Gets tricky with nested lock_kernels.

We can do

	if (preempt_count() - current->lock_depth)

To ignore the bkl contribution to preempt_count.

I think that's even usable in generic code, because all architectures
use lock_depth in the same way.

Re: 2.5.31: modules don't work at all

[Linus Torvalds]

On Mon, 12 Aug 2002, Andrew Morton wrote:
> 
> Gets tricky with nested lock_kernels.

No, lock-kernel already only increments once, at the first lock_kernel. We 
have a totally separate counter for the BKL depth, see <asm/smplock.h>

		Linus

Re: 2.5.31: modules don't work at all

[Andrew Morton]

Linus Torvalds wrote:
> 
> On Mon, 12 Aug 2002, Andrew Morton wrote:
> >
> > Gets tricky with nested lock_kernels.
> 
> No, lock-kernel already only increments once, at the first lock_kernel. We
> have a totally separate counter for the BKL depth, see <asm/smplock.h>
> 

There are eighteen smplock.h's, all different.  At least one (SuperH)
hasn't been converted to preempt.  I'd rather like to decouple the kmap
optimisation work from that mini-quagmire.  It's very easy to do, with

	current->flags |= PF_ATOMIC;
	__copy_to_user(...);
	current->flags &= ~PF_ATOMIC;

and that all works fine.

However, soldiering on leads us to some difficulties.  You're proposing,
effectively, that preempt_count gets shifted left one bit and that bit
zero becomes "has done lock_kernel()".

So bits 0-31 of preempt_count mean "may not be preempted" and "should
not preempt self".  And bits 1-31 of preempt count mean "must not
explicitly call schedule".

Problem is, the semantics of this vary too much between preemptible
and non-preemptible kernels.  Because non-preemptible kernels do
not increment preempt_count in spin_lock().

Maybe my penny hasn't dropped yet, but I tend to feel that the
semantics of my "may_schedule()" below are too fragile for it to
be part of the API.

(Untested, uncompiled code)


 arch/i386/mm/fault.c       |    2 -
 include/asm-i386/smplock.h |   20 +++++++++++++-----
 include/linux/preempt.h    |   49 ++++++++++++++++++++++++++++++++-------------
 3 files changed, 51 insertions, 20 deletions

--- 2.5.31/arch/i386/mm/fault.c~fix-faults	Mon Aug 12 16:14:21 2002
+++ 2.5.31-akpm/arch/i386/mm/fault.c	Mon Aug 12 16:14:21 2002
@@ -192,7 +192,7 @@ asmlinkage void do_page_fault(struct pt_
 	 * If we're in an interrupt, have no user context or are running in an
 	 * atomic region then we must not take the fault..
 	 */
-	if (preempt_count() || !mm)
+	if (!may_schedule() || !mm)
 		goto no_context;
 
 #ifdef CONFIG_X86_REMOTE_DEBUG
--- 2.5.31/include/linux/preempt.h~fix-faults	Mon Aug 12 16:14:21 2002
+++ 2.5.31-akpm/include/linux/preempt.h	Mon Aug 12 16:14:21 2002
@@ -5,29 +5,60 @@
 
 #define preempt_count() (current_thread_info()->preempt_count)
 
+/*
+ * Bit zero of preempt_count means "holds lock_kernel".
+ * So a non-zero value in preempt_count() means "may not be preempted" and a
+ * non-zero value in bits 1-31 means "may not explicitly schedule".
+ */
+
 #define inc_preempt_count() \
 do { \
-	preempt_count()++; \
+	preempt_count() += 2; \
 } while (0)
 
 #define dec_preempt_count() \
 do { \
-	preempt_count()--; \
+	preempt_count() -= 2; \
+} while (0)
+
+#define lock_kernel_enter() \
+do { \
+	preempt_count() |= 1; \
+} while (0)
+
+#define lock_kernel_exit() \
+do { \
+	preempt_count() &= ~1; \
 } while (0)
 
+/*
+ * The semantics of this depend upon CONFIG_PREEMPT.
+ *
+ * With CONFIG_PREEMPT=y, may_schedule() returns false in irq context and
+ * inside spinlocks, and returns true inside lock_kernel().
+ *
+ * With CONFIG_PREEMPT=n, may_schedule() returns false in irq context, returns
+ * true inside spinlocks and returns true inside lock_kernel().
+ *
+ * But may_schedule() will also return false if the task has performed an
+ * explicit inc_preempt_count(), regardless of CONFIG_PREEMPT.  Which is really
+ * the only situation in which may_schedule() is useful.
+ */
+#define may_schedule()	(!(preempt_count() >> 1))
+
 #ifdef CONFIG_PREEMPT
 
 extern void preempt_schedule(void);
 
 #define preempt_disable() \
 do { \
-	inc_preempt_count(); \
+	preempt_count() += 2; \
 	barrier(); \
 } while (0)
 
 #define preempt_enable_no_resched() \
 do { \
-	dec_preempt_count(); \
+	preempt_count() -= 2; \
 	barrier(); \
 } while (0)
 
@@ -44,22 +75,12 @@ do { \
 		preempt_schedule(); \
 } while (0)
 
-#define inc_preempt_count_non_preempt()	do { } while (0)
-#define dec_preempt_count_non_preempt()	do { } while (0)
-
 #else
 
 #define preempt_disable()		do { } while (0)
 #define preempt_enable_no_resched()	do {} while(0)
 #define preempt_enable()		do { } while (0)
 #define preempt_check_resched()		do { } while (0)
-
-/*
- * Sometimes we want to increment the preempt count, but we know that it's
- * already incremented if the kernel is compiled for preemptibility.
- */
-#define inc_preempt_count_non_preempt()	inc_preempt_count()
-#define dec_preempt_count_non_preempt()	dec_preempt_count()
 
 #endif
 
--- 2.5.31/include/asm-i386/smplock.h~fix-faults	Mon Aug 12 16:14:21 2002
+++ 2.5.31-akpm/include/asm-i386/smplock.h	Mon Aug 12 16:15:20 2002
@@ -25,8 +25,10 @@ extern spinlock_t kernel_flag;
  */
 #define release_kernel_lock(task)		\
 do {						\
-	if (unlikely(task->lock_depth >= 0))	\
+	if (unlikely(task->lock_depth >= 0)) {	\
 		spin_unlock(&kernel_flag);	\
+		lock_kernel_exit();		\
+	}					\
 } while (0)
 
 /*
@@ -34,8 +36,10 @@ do {						\
  */
 #define reacquire_kernel_lock(task)		\
 do {						\
-	if (unlikely(task->lock_depth >= 0))	\
+	if (unlikely(task->lock_depth >= 0)) {	\
+		lock_kernel_enter();		\
 		spin_lock(&kernel_flag);	\
+	}					\
 } while (0)
 
 
@@ -49,13 +53,17 @@ do {						\
 static __inline__ void lock_kernel(void)
 {
 #ifdef CONFIG_PREEMPT
-	if (current->lock_depth == -1)
+	if (current->lock_depth == -1) {
+		lock_kernel_enter();
 		spin_lock(&kernel_flag);
+	}
 	++current->lock_depth;
 #else
 #if 1
-	if (!++current->lock_depth)
+	if (!++current->lock_depth) {
+		lock_kernel_enter();
 		spin_lock(&kernel_flag);
+	}
 #else
 	__asm__ __volatile__(
 		"incl %1\n\t"
@@ -73,8 +81,10 @@ static __inline__ void unlock_kernel(voi
 	if (current->lock_depth < 0)
 		BUG();
 #if 1
-	if (--current->lock_depth < 0)
+	if (--current->lock_depth < 0) {
 		spin_unlock(&kernel_flag);
+		lock_kernel_exit();
+	}
 #else
 	__asm__ __volatile__(
 		"decl %1\n\t"

.

Re: 2.5.31: modules don't work at all

[Linus Torvalds]

On Mon, 12 Aug 2002, Andrew Morton wrote:
> Linus Torvalds wrote:
> > 
> > On Mon, 12 Aug 2002, Andrew Morton wrote:
> > >
> > > Gets tricky with nested lock_kernels.
> > 
> > No, lock-kernel already only increments once, at the first lock_kernel. We
> > have a totally separate counter for the BKL depth, see <asm/smplock.h>
> > 
> 
> There are eighteen smplock.h's, all different.  At least one (SuperH)
> hasn't been converted to preempt.

Note that this should all be trivial, and in fact I think everybody shares 
the same smplock.h these days. The x86 smplock.h is 100% C - even though 
it's not actually totally visible because we still have some old asm 
routines visible that are just #ifdef'ed out.

In fact, I'll clean that up a bit to make it clearer.

> However, soldiering on leads us to some difficulties.  You're proposing,
> effectively, that preempt_count gets shifted left one bit and that bit
> zero becomes "has done lock_kernel()".

Actually, on slight introspection I suspect the better answer is to make
the BKL bit somewhere higher up, since BKL is much less interesting than
most spinlocks, and getting increasingly more so. But yes.

			Linus

Re: 2.5.31: modules don't work at all

[Skip Ford]

Andrew Morton wrote:
> 
> (Untested, uncompiled code)

I can't boot with this patch applied.  I use a framebuffer and the
screen goes to graphics mode and then nothing...

If you need me to I can get rid of the graphics and see what happens.

-- 
Skip

Re: 2.5.31: modules don't work at all

[Skip Ford]

Andrew Morton wrote:
> 
> (Untested, uncompiled code)

More information.  With your patch I hit the BUG in kmem_cache_create
at slab.c line 673:

[inside kmem_cache_create]

	/*
	 * Sanity checks... these are all serious usage bugs.
	 */
	if ((!name) ||
		in_interrupt() ||
		(size < BYTES_PER_WORD) ||
		(size > (1<<MAX_OBJ_ORDER)*PAGE_SIZE) ||
		(dtor && !ctor) ||
		(offset < 0 || offset > size))
			BUG();
-- 
Skip

Re: 2.5.31: modules don't work at all

[Andrew Rodland]

On Sun, 11 Aug 2002 22:36:50 -0700
Andrew Morton <akpm@zip.com.au> wrote:
> Skip Ford wrote:
> > If I back out this change to arch/i386/mm/fault.c then modules
> > successfully load.  I have no idea if backing it out causes other
> > problems though.
> > 
> > diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
> > --- a/arch/i386/mm/fault.c      Sat Aug 10 18:42:20 2002
> > +++ b/arch/i386/mm/fault.c      Sat Aug 10 18:42:20 2002
> > @@ -181,10 +181,10 @@
> >         info.si_code = SEGV_MAPERR;
> > 
> >         /*
> > -        * If we're in an interrupt or have no user
> > -        * context, we must not take the fault..
> > +        * If we're in an interrupt, have no user context or are
> > running in an
> > +        * atomic region then we must not take the
> > fault..
> >          */
> > -       if (in_interrupt() || !mm)
> > +       if (preempt_count() || !mm)
> >                 goto no_context;
> > 
> >         down_read(&mm->mmap_sem);
> > 
> 
> Yes, that's the problem.   qm_symbols() is performing copy_to_user()
> inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
> But only if preempt is selected.  The copy_to_user() doesn't work.
> 
> There's nothing illegal about copy_to_user() inside lock_kernel().

Does that mean that the above fix is a legal quick-fix and won't cause
things to fall apart, or does it mean that I shouldn't bother until the
next version?

Re: 2.5.31: modules don't work at all

[Andrew Morton]

Andrew Rodland wrote:
> 
> > > -       if (in_interrupt() || !mm)
> > > +       if (preempt_count() || !mm)
> > >                 goto no_context;
> > >
> > >         down_read(&mm->mmap_sem);
> > >
> >
> > Yes, that's the problem.   qm_symbols() is performing copy_to_user()
> > inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
> > But only if preempt is selected.  The copy_to_user() doesn't work.
> >
> > There's nothing illegal about copy_to_user() inside lock_kernel().
> 
> Does that mean that the above fix is a legal quick-fix and won't cause
> things to fall apart, or does it mean that I shouldn't bother until the
> next version?

That a legal quick-fix.  Or disable CONFIG_PREEMPT.

Re: 2.5.31: modules don't work at all

[Ed Tomlinson]

Andrew Morton wrote:

> Skip Ford wrote:
>> 
>> ...
>> >       I already know that the error that trips insmod occurs at
>> > in modules.c, line 831, when qm_symbols gets an error from
>> > copy_to_user():
>> >
>> >         for (; i < mod->nsyms ; ++i, ++s, vals += 2) {
>> >                 len = strlen(s->name)+1;
>> >                 if (len > bufsize)
>> >                         goto calc_space_needed;
>> >
>> > here------>     if (copy_to_user(strings, s->name, len)
>> >                     || __put_user(s->value, vals+0)
>> >                     || __put_user(space, vals+1))
>> >                         return -EFAULT;
>> >
>> >                 strings += len;
>> >                 bufsize -= len;
>> >                 space += len;
>> >         }
>> >
>> >       The values of strings and s->name are similar in 2.5.30+preempt
>> > (works) and 2.5.31+preempt (does not work).  strings is 0x08______, and
>> > s->name is 0xc0______.
>> 
>> If I back out this change to arch/i386/mm/fault.c then modules
>> successfully load.  I have no idea if backing it out causes other
>> problems though.
>> 
>> diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
>> --- a/arch/i386/mm/fault.c      Sat Aug 10 18:42:20 2002
>> +++ b/arch/i386/mm/fault.c      Sat Aug 10 18:42:20 2002
>> @@ -181,10 +181,10 @@
>>         info.si_code = SEGV_MAPERR;
>> 
>>         /*
>> -        * If we're in an interrupt or have no user
>> -        * context, we must not take the fault..
>> +        * If we're in an interrupt, have no user context or are running
>> in an
>> +        * atomic region then we must not take the fault..
>>          */
>> -       if (in_interrupt() || !mm)
>> +       if (preempt_count() || !mm)
>>                 goto no_context;
>> 
>>         down_read(&mm->mmap_sem);
>> 
> 
> Yes, that's the problem.   qm_symbols() is performing copy_to_user()
> inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
> But only if preempt is selected.  The copy_to_user() doesn't work.
> 
> There's nothing illegal about copy_to_user() inside lock_kernel().
> 
> Linus, we can back out the preempt_count() test in there and
> perform the atomic copy_*_user via a current->flags bit, or
> we can do something else?

I am still seeing this problem with the todays bk current, which includes
the above 'fix'...

Turning off preempt now.

Ed Tomlinson

Re: 2.5.31: modules don't work at all

[Flanigan, Ryan]

>>>>> "Adam" == Adam J Richter <adam@yggdrasil.com> writes:

    Adam> 	Ryan, thanks for suggesting that, as it would have taken
    Adam> me a long time to narrow it down that far!

np. 

    Adam> 	It would help avoid duplication of effort if you could
    Adam> indicate how along you are with this problem.  If you or

just a "test" build w/ PREEMPT enabled is when i noticed it. it
was the only thing i changed in the .config. so ... 
i just wanted to isolate it a bit more before posting.

    Adam> someone else has nailed the problem and is preparing a
    Adam> patch, then there is no point in anyone else trying to
    Adam> duplicate that debugging effort.  On the other hand, if you

i have only put in a few hours on the problem thus far, and plan
to continue tonight by looking to the  "hold atomic kmaps across 
generic_file_read" and "Forward port of get_user_pages() change 
from 2.4" patch by Andrew Morton <akpm@zip.com.au>.  thats my best 
guess thus far.  others might think differently (and they're probably 
right).

    Adam> just noticed CONFIG_PREEMPT was the difference between your
    Adam> configuration and that of someone else who was running
    Adam> 2.5.31 successfully and are not actively debugging the
    Adam> problem, then I'll try to poke at it some more.

please do. im still _slow_ when dealing with these things.

    Adam> 	I already know that the error that trips insmod occurs at
    Adam> in modules.c, line 831, when qm_symbols gets an error from
    Adam> copy_to_user():

agreed and thanks for the info!