From: Andrew Morton <akpm@linux-foundation.org> Date: 2008-01-17 22:55:50
On Thu, 17 Jan 2008 23:15:27 +0100
Mariusz Kozlowski [off-list ref] wrote:
Hello,
The script below kills powerpc. oopses get longer and more
wonderful with every next 'cated' file.
ppc32.
/proc/<pid>/task/<pid>/pagemap seems to be the cause of oops. The
important thing is that it oopses for random (that is not first in a row)
process from /proc. So not every 'cat /proc/<pid>/task/<pid>/pagemap'
causes an oops.
I could try to bisect this but this powerpc box is iMac G3 (cpu at 400MHz)
and this will take time. So any hints appreciated.
Regards,
Mariusz
script:
---------
#!/bin/bash
for i in `find /proc/*/ -readable -type f`; do
echo -n "cat $i > /dev/null ... ";
logger -t proc_loop $i;
sync;
cat $i > /dev/null;
echo "done";
done
----------
syslog:
proc_loop: /proc/3731/task/3731/pagemap
kernel: BUG: sleeping function called from invalid context at fs/proc/task_mmu.c:554
kernel: in_atomic():1, irqs_disabled():0
kernel: Call Trace:
kernel: [cf1cddf0] [c000840c] show_stack+0x3c/0x194 (unreliable)
kernel: [cf1cde20] [c002b2ec] __might_sleep+0xf4/0x108
kernel: [cf1cde30] [c00d2d54] add_to_pagemap+0x40/0x11c
kernel: [cf1cde50] [c00d2f44] pagemap_pte_range+0xa8/0x10c
kernel: [cf1cde70] [c0081b30] walk_page_range+0x148/0x23c
kernel: [cf1cdeb0] [c00d3104] pagemap_read+0x15c/0x244
kernel: [cf1cdef0] [c0092144] vfs_read+0xc4/0x16c
kernel: [cf1cdf10] [c009261c] sys_read+0x4c/0x90
kernel: [cf1cdf40] [c001328c] ret_from_syscall+0x0/0x40
It's not really an oops - it's a warning. add_to_pagemap() is doing a
put_user() inside pagemap_pte_range->pte_offset_map->kmap_atomic.
A known bug, I'm afraid.
How to fix?
- double-buffer the data to be copied to userspace or
- take a local copy of the pte page then work on that instead or
- play copy_to_user_inatomic() tricks.
It would be really nice to get the maps4 stuff merged this time around but
it is looking unlikely.
kernel: --- Exception: c01 at 0xff5a364
kernel: LR = 0x10002f60
kernel: BUG: scheduling while atomic: cat/8929/0x00000002
kernel: Call Trace:
kernel: [cf1cde90] [c000840c] show_stack+0x3c/0x194 (unreliable)
kernel: [cf1cdec0] [c002db24] __schedule_bug+0x64/0x78
kernel: [cf1cdee0] [c027207c] schedule+0x304/0x32c
kernel: [cf1cdf40] [c0013a5c] recheck+0x0/0x28
kernel: --- Exception: c01 at 0xff5a364
kernel: LR = 0x10002f60
kernel: BUG: scheduling while atomic: cat/8929/0x00000007
kernel: Call Trace:
kernel: [cf1cde90] [c000840c] show_stack+0x3c/0x194 (unreliable)
kernel: [cf1cdec0] [c002db24] __schedule_bug+0x64/0x78
kernel: [cf1cdee0] [c027207c] schedule+0x304/0x32c
kernel: [cf1cdf40] [c0013a5c] recheck+0x0/0x28
kernel: --- Exception: c01 at 0xff5a364
kernel: LR = 0x10002f60
kernel: BUG: scheduling while atomic: cat/8929/0x00000009
kernel: Call Trace:
kernel: [cf1cde90] [c000840c] show_stack+0x3c/0x194 (unreliable)
kernel: [cf1cdec0] [c002db24] __schedule_bug+0x64/0x78
kernel: [cf1cdee0] [c027207c] schedule+0x304/0x32c
kernel: [cf1cdf40] [c0013a5c] recheck+0x0/0x28
kernel: --- Exception: c01 at 0xff5a364
kernel: LR = 0x10002f60
kernel: Page fault in user mode with in_atomic() = 1 mm = cf2765a0
kernel: NIP = ff0cbc8 MSR = 4000f932
kernel: Oops: Weird page fault, sig: 11 [#3]
hm. Not sure how that happened. The arch code thinks we're running in
user mode.
It's not really an oops - it's a warning. add_to_pagemap() is doing a
put_user() inside pagemap_pte_range->pte_offset_map->kmap_atomic.
A known bug, I'm afraid.
How to fix?
- double-buffer the data to be copied to userspace or
- take a local copy of the pte page then work on that instead or
- play copy_to_user_inatomic() tricks.
Hmm, this fell off my radar. How about something like this as a minimal
fix (untested as -mm is a complete doorstop for me at the moment)?
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 17:29:21 2008 -0600
It's not really an oops - it's a warning. add_to_pagemap() is doing a
put_user() inside pagemap_pte_range->pte_offset_map->kmap_atomic.
A known bug, I'm afraid.
How to fix?
- double-buffer the data to be copied to userspace or
- take a local copy of the pte page then work on that instead or
- play copy_to_user_inatomic() tricks.
Hmm, this fell off my radar. How about something like this as a minimal
fix (untested as -mm is a complete doorstop for me at the moment)?
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 17:29:21 2008 -0600
It's not really an oops - it's a warning. add_to_pagemap() is doing a
put_user() inside pagemap_pte_range->pte_offset_map->kmap_atomic.
A known bug, I'm afraid.
How to fix?
- double-buffer the data to be copied to userspace or
- take a local copy of the pte page then work on that instead or
- play copy_to_user_inatomic() tricks.
Hmm, this fell off my radar. How about something like this as a minimal
fix (untested as -mm is a complete doorstop for me at the moment)?
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 17:29:21 2008 -0600
Good point, it really can be taht simple.
Do we need the ifdef? pte_offset_map/pte_unmap should be super-cheap on
!CONFIG_HIGHPTE builds.
In that case, pte_unmap is free, pte_offset_map is just a bit of math.
So yeah, we can simplify this. How about:
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 18:11:13 2008 -0600
From: Andrew Morton <akpm@linux-foundation.org> Date: 2008-01-18 00:30:15
On Thu, 17 Jan 2008 18:12:48 -0600
Matt Mackall [off-list ref] wrote:
quoted hunk
quoted
Do we need the ifdef? pte_offset_map/pte_unmap should be super-cheap on
!CONFIG_HIGHPTE builds.
In that case, pte_unmap is free, pte_offset_map is just a bit of math.
So yeah, we can simplify this. How about:
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 18:11:13 2008 -0600
Do we need `offset' at all?
You have
static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
void *private)
{
struct pagemapread *pm = private;
pte_t *pte;
int offset = 0, err = 0;
for (; addr != end; offset++, addr += PAGE_SIZE) {
u64 pfn = PM_NOT_PRESENT;
pte = pte_offset_map(pmd, addr);
if (is_swap_pte(pte[offset]))
pfn = swap_pte_to_pagemap_entry(pte[offset]);
else if (pte_present(pte[offset]))
pfn = pte_pfn(pte[offset]);
pte_unmap(pte);
err = add_to_pagemap(addr, pfn, pm);
if (err)
return err;
}
cond_resched();
return err;
}
but I think we just do s/pte[offset]/*pte/. The virtual address should be
the only thing we need to increment as we walk across the addresses here?
From: Matt Mackall <hidden> Date: 2008-01-18 00:47:58
On Thu, 2008-01-17 at 16:29 -0800, Andrew Morton wrote:
Do we need `offset' at all?
Looks like no.
I wonder if there's a good argument for adding a pte_offset_val() which
would let us do:
pteval = pte_offset_val(pmd, addr);
and shrink the map/unmap window and overhead here and possibly
elsewhere?
Anyway, updated but still untested patch now with revealing comment:
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 18:45:57 2008 -0600
@@ -584,18 +585,19 @@pte_t*pte;interr=0;-pte=pte_offset_map(pmd,addr);-for(;addr!=end;pte++,addr+=PAGE_SIZE){+for(;addr!=end;addr+=PAGE_SIZE){u64pfn=PM_NOT_PRESENT;+pte=pte_offset_map(pmd,addr);if(is_swap_pte(*pte))pfn=swap_pte_to_pagemap_entry(*pte);elseif(pte_present(*pte))pfn=pte_pfn(*pte);+/* unmap so we're not in atomic when we copy to userspace */+pte_unmap(pte);err=add_to_pagemap(addr,pfn,pm);if(err)returnerr;}-pte_unmap(pte-1);cond_resched();
--
Mathematics is the supreme nostalgia of our time.
From: Andrew Morton <akpm@linux-foundation.org> Date: 2008-01-18 01:08:34
On Thu, 17 Jan 2008 18:47:17 -0600
Matt Mackall [off-list ref] wrote:
quoted hunk
On Thu, 2008-01-17 at 16:29 -0800, Andrew Morton wrote:
quoted
Do we need `offset' at all?
Looks like no.
I wonder if there's a good argument for adding a pte_offset_val() which
would let us do:
pteval = pte_offset_val(pmd, addr);
and shrink the map/unmap window and overhead here and possibly
elsewhere?
Anyway, updated but still untested patch now with revealing comment:
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 18:45:57 2008 -0600
@@ -584,18 +585,19 @@pte_t*pte;interr=0;-pte=pte_offset_map(pmd,addr);-for(;addr!=end;pte++,addr+=PAGE_SIZE){+for(;addr!=end;addr+=PAGE_SIZE){u64pfn=PM_NOT_PRESENT;+pte=pte_offset_map(pmd,addr);if(is_swap_pte(*pte))pfn=swap_pte_to_pagemap_entry(*pte);elseif(pte_present(*pte))pfn=pte_pfn(*pte);+/* unmap so we're not in atomic when we copy to userspace */+pte_unmap(pte);err=add_to_pagemap(addr,pfn,pm);if(err)returnerr;}-pte_unmap(pte-1);cond_resched();
That worked out nicely.
Wasn't the old code potentially pte_unmap()ping the wrong address? If we
enter with addr==end?
Looks like no.
I wonder if there's a good argument for adding a pte_offset_val() which
would let us do:
pteval = pte_offset_val(pmd, addr);
and shrink the map/unmap window and overhead here and possibly
elsewhere?
Anyway, updated but still untested patch now with revealing comment:
I patched the ppc32 kernel with this and run tests on /proc.
This patch helps. No more BUGs and oopses :)
Thanks,
Mariusz
quoted hunk
diff -r 5595adaea70f fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c Thu Jan 17 13:26:54 2008 -0600+++ b/fs/proc/task_mmu.c Thu Jan 17 18:45:57 2008 -0600
@@ -584,18 +585,19 @@pte_t*pte;interr=0;-pte=pte_offset_map(pmd,addr);-for(;addr!=end;pte++,addr+=PAGE_SIZE){+for(;addr!=end;addr+=PAGE_SIZE){u64pfn=PM_NOT_PRESENT;+pte=pte_offset_map(pmd,addr);if(is_swap_pte(*pte))pfn=swap_pte_to_pagemap_entry(*pte);elseif(pte_present(*pte))pfn=pte_pfn(*pte);+/* unmap so we're not in atomic when we copy to userspace */+pte_unmap(pte);err=add_to_pagemap(addr,pfn,pm);if(err)returnerr;}-pte_unmap(pte-1);cond_resched();
From: Matt Mackall <hidden> Date: 2008-01-18 17:34:09
On Fri, 2008-01-18 at 18:23 +0100, Mariusz Kozlowski wrote:
Hello,
quoted
quoted
Do we need `offset' at all?
Looks like no.
I wonder if there's a good argument for adding a pte_offset_val() which
would let us do:
pteval = pte_offset_val(pmd, addr);
and shrink the map/unmap window and overhead here and possibly
elsewhere?
Anyway, updated but still untested patch now with revealing comment:
I patched the ppc32 kernel with this and run tests on /proc.
This patch helps. No more BUGs and oopses :)
Thanks, Andrew's already queued it up.
--
Mathematics is the supreme nostalgia of our time.