mremap sleeping in incorrect context
Subsystems (2)
Just had that in my log, running 2.6.0-test2. I'm not familiar with this code, but Arjan says this is an old problem that was fixed ages ago, maybe the fix was lost ?
Debug: sleeping function called from invalid context at mm/page_alloc.c:545
Call trace: [c000c1a8] dump_stack+0x18/0x28 [c0021044] __might_sleep+0x6c/0x84 [c004e33c] __alloc_pages+0x338/0x33c [c0014940] pte_alloc_one+0x24/0x160 [c005c224] pte_alloc_map+0x88/0x2a8 [c0065a40] move_one_page+0xd0/0x2a4 [c0065c6c] move_page_tables+0x58/0xb8 [c0065d60] move_vma+0x94/0x824 [c00666f4] do_mremap+0x204/0x468 [c00669d4] sys_mremap+0x7c/0xcc [c0007a5c] ret_from_syscall+0x0/0x44
Ben.
Quoted 7 lines · Benjamin Herrenschmidt
Just had that in my log, running 2.6.0-test2. I'm not familiar with this code, but Arjan says this is an old problem that was fixed ages ago, maybe the fix was lost ?
Debug: sleeping function called from invalid context at
mm/page_alloc.c:545
Quoted 11 lines
Call trace: [c000c1a8] dump_stack+0x18/0x28 [c0021044] __might_sleep+0x6c/0x84 [c004e33c] __alloc_pages+0x338/0x33c [c0014940] pte_alloc_one+0x24/0x160 [c005c224] pte_alloc_map+0x88/0x2a8 [c0065a40] move_one_page+0xd0/0x2a4 [c0065c6c] move_page_tables+0x58/0xb8 [c0065d60] move_vma+0x94/0x824 [c00666f4] do_mremap+0x204/0x468 [c00669d4] sys_mremap+0x7c/0xcc
oops. What are your CONFIG_HIGHMEM and CONFIG_HIGHPTE settings there?
Quoted 2 lines
oops. What are your
CONFIG_HIGHMEMandCONFIG_HIGHPTEsettings there?
this is on ppc32, HIGHPTE doesn't exist, HIGHMEM is enabled (1Gb of RAM)
Ben.
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 3 lines
oops. What are your
CONFIG_HIGHMEMandCONFIG_HIGHPTEsettings there?this is on ppc32, HIGHPTE doesn't exist, HIGHMEM is enabled (1Gb of RAM)
OK, thanks. Seems that I made a little bug. This should fix it. With a changelog like this, it _has_ to be right ;)
move_one_page() is awkward. It grabs an atomic_kmap of the source pte (because it needs to know if there's really a page there) and then it needs to allocate a pte for the dest. But it cannot allocate the dest pte while holding the src's atomic kmap.
So it performs this little dance peeking at pagetables to predict if alloc_one_pte_map() might need to perform a pte page allocation.
When I wrote this code I made it conditional on CONFIG_HIGHPTE. But that was
bogus: even in the !CONFIG_HIGHPTE case, get_one_pte_map_nested() will run atomic_kmap() against the pte page, which disables preemption.
Net effect: with CONFIG_HIGHMEM && !CONFIG_HIGHPTE we can end up performing a GFP_KERNEL pte page allocation while preemption is disabled. It triggers a might_sleep() warning and indeed is buggy.
So the patch removes the conditionality: even in the !CONFIG_HIGHPTE case we still do the pagetable peek and drop the kmap if necessary.
(Arguably, we shouldn't be performing the atomic_kmap() at all if !CONFIG_HIGHPTE: all it does is a pointless preemption disable).
(Arguably, kmap_atomic() should not be disabling preemption if the target page is not highmem. But we're doing it anyway at present for consistency (ie: debug coverage) and because the filemap.c pagecache copying functions rely on kmap_atomic() disabling do_no_page() for all pages: see do_no_page()'s use of in_atomic()).
25-akpm/mm/mremap.c | 4 ---- 1 files changed, 4 deletions(-)
mm/mremap.c renamed from mm/mremap.c~mremap-atomicity-fix +0 -4
@@ -56,7 +56,6 @@ end:return pte; } -#ifdef CONFIG_HIGHPTE /* Save a few cycles on the sane machines */ static inline int page_table_present (struct mm_struct *mm,unsigned long addr) {pgd_t *pgd;
@@ -68,9 +67,6 @@ static inline int page_table_present(str pmd =pmd_offset (pgd, addr);return pmd_present (*pmd); } -#else -#define page_table_present (mm, addr) (1) -#endif static inline pte_t *alloc_one_pte_map (struct mm_struct *mm,unsigned long addr) {
_
Quoted 11 lines · Andrew Morton
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 3 lines
oops. What are your
CONFIG_HIGHMEMandCONFIG_HIGHPTEsettings there?this is on ppc32, HIGHPTE doesn't exist, HIGHMEM is enabled (1Gb of RAM)
OK, thanks. Seems that I made a little bug. This should fix it. With a changelog like this, it _has_ to be right ;)
Thanks, I'll apply here and let you know if this warning is really gone after a couple of days of use...
Ben.