Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Manfred Spraul]

> This is interesting. [Assumes UP Athlon - correct]
> Note that all BUGs out in highmem.h:95 (kmap_atomic)
> and that test is only on if you have enabled HIGHMEM_DEBUG
> [my analyze is done with a 2.4.10-pre2 kernel, but I checked with
> later patches and I do not think they fix it either...]
>
> The preemptive kernel puts more SMP stress on the kernel than
> running with multiple CPUs.
>
> So this might be a potential bug in the kernel proper, running with
> a SMP computer.

No.
It seems to be a missing ctx_sw_off() in highmem.h:
kmap_atomic uses a per-cpu variable, thus ctx_sw_off() is needed in
kmap_atomic, and ctx_sw_on() in kunmap_atomic().

--
    Manfred

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Robert Love]

On Sat, 2001-09-08 at 19:11, Manfred Spraul wrote:
> No.
> It seems to be a missing ctx_sw_off() in highmem.h:
> kmap_atomic uses a per-cpu variable, thus ctx_sw_off() is needed in
> kmap_atomic, and ctx_sw_on() in kunmap_atomic().

in my tree, kmap_atomic and kunmap_atomic are just defined to
kmap/kunmap.  are you suggesting something like this?

#define kmap_atomic(page,idx)	ctx_sw_off(); kmap(page);
#define kunmap_atomic(page,idx)	ctx_sw_on(); kunmap(page);

-- 
Robert M. Love
rml at ufl.edu
rml at tech9.net

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Manfred Spraul]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]


> #define kmap_atomic(page,idx) ctx_sw_off(); kmap(page);
> #define kunmap_atomic(page,idx) ctx_sw_on(); kunmap(page);
>
No. kmap_atomic is called from interrupt context, and kmap calls
schedule().

I thought about the attached patch (completely untested).

--
    Manfred

[-- Attachment #2: patch-untested --]
[-- Type: application/octet-stream, Size: 438 bytes --]

--- highmem.h.prev	Sun Sep  9 08:59:04 2001
+++ highmem.h	Sun Sep  9 09:00:07 2001
@@ -88,6 +88,7 @@
 	if (page < highmem_start_page)
 		return page_address(page);
 
+	ctx_sw_off();
 	idx = type + KM_TYPE_NR*smp_processor_id();
 	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
 #if HIGHMEM_DEBUG
@@ -119,6 +120,7 @@
 	pte_clear(kmap_pte-idx);
 	__flush_tlb_one(vaddr);
 #endif
+	ctx_sw_on();
 }
 
 #endif /* __KERNEL__ */

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Pavel Machek]

Hi!

> > #define kmap_atomic(page,idx) ctx_sw_off(); kmap(page);
> > #define kunmap_atomic(page,idx) ctx_sw_on(); kunmap(page);
> >
> No. kmap_atomic is called from interrupt context, and kmap calls
> schedule().
> 
> I thought about the attached patch (completely untested).

is it legal to kmap_atomic(a,b); kmap_atomic(c,d); kunmap_atomic(a,b); ?
If so, your patch may need some ounting....
								Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Manfred Spraul]

>
> is it legal to kmap_atomic(a,b); kmap_atomic(c,d); kunmap_atomic(a,b);
?
>
Yes, that's legal - just think about one kmap_atomic from process
context, and another one in irq context.

> If so, your patch may need some ounting....
> Pavel

I hope ctx_sw_off does internal counting, correct?

--
    Manfred

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Robert Love]

On Fri, 2001-09-14 at 05:15, Pavel Machek wrote:
> is it legal to kmap_atomic(a,b); kmap_atomic(c,d); kunmap_atomic(a,b); ?
> If so, your patch may need some ounting....

ctx_sw_on and ctx_sw_off use a recursive spinlock, so the calls to
kunmap_atomic won't drop the slock until the last call.

-- 
Robert M. Love
rml at ufl.edu
rml at tech9.net

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Robert Love]

On Mon, 2001-09-17 at 18:40, Manfred Spraul wrote:
> > is it legal to kmap_atomic(a,b); kmap_atomic(c,d); kunmap_atomic(a,b);
>
> Yes, that's legal - just think about one kmap_atomic from process
> context, and another one in irq context.
> 
> > If so, your patch may need some ounting....
> > Pavel
> 
> I hope ctx_sw_off does internal counting, correct?

yes, ctx_sw_off atomically increments a counter and ctx_sw_on
atomic_dec_and_test()s it.

-- 
Robert M. Love
rml at ufl.edu
rml at tech9.net

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Arjan Filius]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 4086 bytes --]

Hi,

On Sun, 9 Sep 2001, george anzinger wrote:

> If the page it is the correct one, when it is found mapped, the code
> should just exit, not BUG() IHMO.


I'll try the ac10 +preempt, see what happens.

>
> George
>
>
> Roger Larsson wrote:
> >
> > Hi,
> >
> > This is interesting. [Assumes UP Athlon - correct]
> > Note that all BUGs out in highmem.h:95 (kmap_atomic)
> > and that test is only on if you have enabled HIGHMEM_DEBUG
> > [my analyze is done with a 2.4.10-pre2 kernel, but I checked with
> > later patches and I do not think they fix it either...]
> >
> > The preemptive kernel puts more SMP stress on the kernel than
> > running with multiple CPUs.
> >
> > So this might be a potential bug in the kernel proper, running with
> > a SMP computer.
> >
> > If I understand the bug correctly, a process gets a page fault.
> > Starts to map in the page. But before the final part it checks -
> > and the page is already there!!! Correct?
> >
> > On Saturday den 8 September 2001 19:33, Arjan Filius wrote:
> > > Hello Robert,
> > >
> > >
> > > I tried 2.4.10-pre4 with patch-rml-2.4.10-pre4-preempt-kernel-1.
> > > But it seems to hit highmem (see below) (i do have 1.5GB ram)
> > > 2.4.10-pre4 plain runs just fine.
> > >
> > > With the kernel option mem=850M the patched kernel boots an seems to run
> > > fine. However i didn't do any stress testing yet, but i still notice
> > > hickups while playing mp3 files at -10 nice level with mpg123 on a 1.1GHz
> > > Athlon, and removing for example a _large_ file (reiser-on-lvm).
> > >
> > > My syslog output with highmem:
> > >
> > > Sep  8 18:10:16 sjoerd kernel: kernel BUG at
> > > /usr/src/linux-2.4.10-pre4/include/asm/highmem.h:95! Sep  8 18:10:16 sjoerd
> > > kernel: invalid operand: 0000
> > > Sep  8 18:10:16 sjoerd kernel: CPU:    0
> > > Sep  8 18:10:16 sjoerd kernel: EIP:    0010:[do_wp_page+636/1088]
> > > [- - -]
> > > sjoerd kernel: Call Trace: [handle_mm_fault+141/224]
> > > [do_page_fault+375/1136] [do_page_fault+0/1136] [__mmdrop+58/64]
> > > [do_exit+595/640] Sep  8 18:10:16 sjoerd kernel:    [error_code+52/64]
> >
> > Lets look at this example. You need to add some inline functions...
> >
> > handle_mm_fault
> >         takes the mm->page_table_lock [this should prevent reschedules]
> >         allocs pmd
> >         allocs pte
> >         handle_pte_fault(...)
> > handle_pte_fault [inline, most likely path]
> >         pte is present
> >         it is a write access
> >         but the pte is not writeable  - call do_wp_page
> > do_wp_page
> >         plays some games with the lock...
> >         finally calls copy_cow_page [inline] with the page_table_lock
> >         UNLOCKED!
> > copy_cow_page
> >         calls clear_user_highpage or copy_user_highpage
> > both clear_user_highpage and copy_user_highpage
> >         calls kmap_atomic
> > kmap_atomic
> >         page is a highmem page
> >         but during the time this process was unlocked some other
> >         thread has allocated the page in question... BUG out.
> >
> > So somewere between the UNLOCK (might be a lot later) and the
> > BUG test in kmap_atomic the process running in kernel got preempted.
> > (most likely during the page copy since it will take some time)
> >
> > Another process (thread) started to run - hit the same page fault
> > but succeeded in its alloc.
> >
> > Back to the first process it continues, finally checks - the page
> > is there... and BUGS.
> >
> > Note that this can happen in a pure SMP kernel.
> >
> > But let the processes (threads) run on two CPUs. And let the
> > first get an interrupt/bh after unlock - the other can pass
> > and add the page before the first one can continue - same
> > result!
> >
> > /RogerL
> >
> > --
> > Roger Larsson
> > Skellefteå
> > Sweden
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
>

-- 
Arjan Filius
mailto:iafilius@xs4all.nl

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[george anzinger]

If the page it is the correct one, when it is found mapped, the code
should just exit, not BUG() IHMO.

George


Roger Larsson wrote:
> 
> Hi,
> 
> This is interesting. [Assumes UP Athlon - correct]
> Note that all BUGs out in highmem.h:95 (kmap_atomic)
> and that test is only on if you have enabled HIGHMEM_DEBUG
> [my analyze is done with a 2.4.10-pre2 kernel, but I checked with
> later patches and I do not think they fix it either...]
> 
> The preemptive kernel puts more SMP stress on the kernel than
> running with multiple CPUs.
> 
> So this might be a potential bug in the kernel proper, running with
> a SMP computer.
> 
> If I understand the bug correctly, a process gets a page fault.
> Starts to map in the page. But before the final part it checks -
> and the page is already there!!! Correct?
> 
> On Saturday den 8 September 2001 19:33, Arjan Filius wrote:
> > Hello Robert,
> >
> >
> > I tried 2.4.10-pre4 with patch-rml-2.4.10-pre4-preempt-kernel-1.
> > But it seems to hit highmem (see below) (i do have 1.5GB ram)
> > 2.4.10-pre4 plain runs just fine.
> >
> > With the kernel option mem=850M the patched kernel boots an seems to run
> > fine. However i didn't do any stress testing yet, but i still notice
> > hickups while playing mp3 files at -10 nice level with mpg123 on a 1.1GHz
> > Athlon, and removing for example a _large_ file (reiser-on-lvm).
> >
> > My syslog output with highmem:
> >
> > Sep  8 18:10:16 sjoerd kernel: kernel BUG at
> > /usr/src/linux-2.4.10-pre4/include/asm/highmem.h:95! Sep  8 18:10:16 sjoerd
> > kernel: invalid operand: 0000
> > Sep  8 18:10:16 sjoerd kernel: CPU:    0
> > Sep  8 18:10:16 sjoerd kernel: EIP:    0010:[do_wp_page+636/1088]
> > [- - -]
> > sjoerd kernel: Call Trace: [handle_mm_fault+141/224]
> > [do_page_fault+375/1136] [do_page_fault+0/1136] [__mmdrop+58/64]
> > [do_exit+595/640] Sep  8 18:10:16 sjoerd kernel:    [error_code+52/64]
> 
> Lets look at this example. You need to add some inline functions...
> 
> handle_mm_fault
>         takes the mm->page_table_lock [this should prevent reschedules]
>         allocs pmd
>         allocs pte
>         handle_pte_fault(...)
> handle_pte_fault [inline, most likely path]
>         pte is present
>         it is a write access
>         but the pte is not writeable  - call do_wp_page
> do_wp_page
>         plays some games with the lock...
>         finally calls copy_cow_page [inline] with the page_table_lock
>         UNLOCKED!
> copy_cow_page
>         calls clear_user_highpage or copy_user_highpage
> both clear_user_highpage and copy_user_highpage
>         calls kmap_atomic
> kmap_atomic
>         page is a highmem page
>         but during the time this process was unlocked some other
>         thread has allocated the page in question... BUG out.
> 
> So somewere between the UNLOCK (might be a lot later) and the
> BUG test in kmap_atomic the process running in kernel got preempted.
> (most likely during the page copy since it will take some time)
> 
> Another process (thread) started to run - hit the same page fault
> but succeeded in its alloc.
> 
> Back to the first process it continues, finally checks - the page
> is there... and BUGS.
> 
> Note that this can happen in a pure SMP kernel.
> 
> But let the processes (threads) run on two CPUs. And let the
> first get an interrupt/bh after unlock - the other can pass
> and add the page before the first one can continue - same
> result!
> 
> /RogerL
> 
> --
> Roger Larsson
> Skellefteå
> Sweden
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Arjan Filius]

Hello Roger,

On Sat, 8 Sep 2001, Roger Larsson wrote:

> Hi,
>
> This is interesting. [Assumes UP Athlon - correct]

UP Athlon, and compiled as UP (as always).
I haven't tested my system with an SMP kernel for a long while.



> Note that all BUGs out in highmem.h:95 (kmap_atomic)
> and that test is only on if you have enabled HIGHMEM_DEBUG
It seems to be on indeed.

> [my analyze is done with a 2.4.10-pre2 kernel, but I checked with
> later patches and I do not think they fix it either...]
>
> The preemptive kernel puts more SMP stress on the kernel than
> running with multiple CPUs.
>
> So this might be a potential bug in the kernel proper, running with
> a SMP computer.

>
> If I understand the bug correctly, a process gets a page fault.
> Starts to map in the page. But before the final part it checks -
> and the page is already there!!! Correct?

ehh.. Should compiling SMP on UP (just for test) trigger this?


Greatings,


>
> On Saturday den 8 September 2001 19:33, Arjan Filius wrote:
> > Hello Robert,
> >
> >
> > I tried 2.4.10-pre4 with patch-rml-2.4.10-pre4-preempt-kernel-1.
> > But it seems to hit highmem (see below) (i do have 1.5GB ram)
> > 2.4.10-pre4 plain runs just fine.
> >
> > With the kernel option mem=850M the patched kernel boots an seems to run
> > fine. However i didn't do any stress testing yet, but i still notice
> > hickups while playing mp3 files at -10 nice level with mpg123 on a 1.1GHz
> > Athlon, and removing for example a _large_ file (reiser-on-lvm).
> >
> > My syslog output with highmem:
> >
> > Sep  8 18:10:16 sjoerd kernel: kernel BUG at
> > /usr/src/linux-2.4.10-pre4/include/asm/highmem.h:95! Sep  8 18:10:16 sjoerd
> > kernel: invalid operand: 0000
> > Sep  8 18:10:16 sjoerd kernel: CPU:    0
> > Sep  8 18:10:16 sjoerd kernel: EIP:    0010:[do_wp_page+636/1088]
> > [- - -]
> > sjoerd kernel: Call Trace: [handle_mm_fault+141/224]
> > [do_page_fault+375/1136] [do_page_fault+0/1136] [__mmdrop+58/64]
> > [do_exit+595/640] Sep  8 18:10:16 sjoerd kernel:    [error_code+52/64]
>
> Lets look at this example. You need to add some inline functions...
>
> handle_mm_fault
> 	takes the mm->page_table_lock [this should prevent reschedules]
> 	allocs pmd
> 	allocs pte
> 	handle_pte_fault(...)
> handle_pte_fault [inline, most likely path]
> 	pte is present
> 	it is a write access
> 	but the pte is not writeable  - call do_wp_page
> do_wp_page
> 	plays some games with the lock...
> 	finally calls copy_cow_page [inline] with the page_table_lock
> 	UNLOCKED!
> copy_cow_page
> 	calls clear_user_highpage or copy_user_highpage
> both clear_user_highpage and copy_user_highpage
> 	calls kmap_atomic
> kmap_atomic
> 	page is a highmem page
> 	but during the time this process was unlocked some other
> 	thread has allocated the page in question... BUG out.
>
> So somewere between the UNLOCK (might be a lot later) and the
> BUG test in kmap_atomic the process running in kernel got preempted.
> (most likely during the page copy since it will take some time)
>
> Another process (thread) started to run - hit the same page fault
> but succeeded in its alloc.
>
> Back to the first process it continues, finally checks - the page
> is there... and BUGS.
>
> Note that this can happen in a pure SMP kernel.
>
> But let the processes (threads) run on two CPUs. And let the
> first get an interrupt/bh after unlock - the other can pass
> and add the page before the first one can continue - same
> result!
>
> /RogerL
>
>

-- 
Arjan Filius
mailto:iafilius@xs4all.nl

[SMP lock BUG?] Re: Feedback on preemptible kernel patch

[Roger Larsson]

Hi,

This is interesting. [Assumes UP Athlon - correct]
Note that all BUGs out in highmem.h:95 (kmap_atomic)
and that test is only on if you have enabled HIGHMEM_DEBUG
[my analyze is done with a 2.4.10-pre2 kernel, but I checked with
later patches and I do not think they fix it either...]

The preemptive kernel puts more SMP stress on the kernel than
running with multiple CPUs.

So this might be a potential bug in the kernel proper, running with
a SMP computer.

If I understand the bug correctly, a process gets a page fault.
Starts to map in the page. But before the final part it checks -
and the page is already there!!! Correct?

On Saturday den 8 September 2001 19:33, Arjan Filius wrote:
> Hello Robert,
>
>
> I tried 2.4.10-pre4 with patch-rml-2.4.10-pre4-preempt-kernel-1.
> But it seems to hit highmem (see below) (i do have 1.5GB ram)
> 2.4.10-pre4 plain runs just fine.
>
> With the kernel option mem=850M the patched kernel boots an seems to run
> fine. However i didn't do any stress testing yet, but i still notice
> hickups while playing mp3 files at -10 nice level with mpg123 on a 1.1GHz
> Athlon, and removing for example a _large_ file (reiser-on-lvm).
>
> My syslog output with highmem:
>
> Sep  8 18:10:16 sjoerd kernel: kernel BUG at
> /usr/src/linux-2.4.10-pre4/include/asm/highmem.h:95! Sep  8 18:10:16 sjoerd
> kernel: invalid operand: 0000
> Sep  8 18:10:16 sjoerd kernel: CPU:    0
> Sep  8 18:10:16 sjoerd kernel: EIP:    0010:[do_wp_page+636/1088]
> [- - -]
> sjoerd kernel: Call Trace: [handle_mm_fault+141/224]
> [do_page_fault+375/1136] [do_page_fault+0/1136] [__mmdrop+58/64]
> [do_exit+595/640] Sep  8 18:10:16 sjoerd kernel:    [error_code+52/64]

Lets look at this example. You need to add some inline functions...

handle_mm_fault
	takes the mm->page_table_lock [this should prevent reschedules]
	allocs pmd
	allocs pte
	handle_pte_fault(...)
handle_pte_fault [inline, most likely path]
	pte is present
	it is a write access
	but the pte is not writeable  - call do_wp_page
do_wp_page
	plays some games with the lock...
	finally calls copy_cow_page [inline] with the page_table_lock
	UNLOCKED!
copy_cow_page
	calls clear_user_highpage or copy_user_highpage
both clear_user_highpage and copy_user_highpage
	calls kmap_atomic
kmap_atomic
	page is a highmem page
	but during the time this process was unlocked some other
	thread has allocated the page in question... BUG out.

So somewere between the UNLOCK (might be a lot later) and the
BUG test in kmap_atomic the process running in kernel got preempted.
(most likely during the page copy since it will take some time)

Another process (thread) started to run - hit the same page fault
but succeeded in its alloc.

Back to the first process it continues, finally checks - the page
is there... and BUGS.

Note that this can happen in a pure SMP kernel.

But let the processes (threads) run on two CPUs. And let the
first get an interrupt/bh after unlock - the other can pass
and add the page before the first one can continue - same
result!

/RogerL

-- 
Roger Larsson
Skellefteå
Sweden