[patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[akpm, Andrew Morton]

See http://bugzilla.kernel.org/show_bug.cgi?id=8928

I think it makes sense to permit a non-BUGging get_zeroed_page(GFP_ATOMIC)
from interrupt context.

We could fix this in several places, but I do think we want to keep the sanity
checks in kmap_atomic() even for non-highmem pages, so that people who are
testing new code on non-highmem machines get their bugs detected earlier.

Cc: Thomas Jarosch <thomas.jarosch@intra2net.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/page_alloc.c |   26 +++++++++++++++++++-------
 1 files changed, 19 insertions(+), 7 deletions(-)

diff -puN mm/page_alloc.c~alloc_pages-permit-get_zeroed_pagegfp_atomic-from-interrupt-context mm/page_alloc.c
--- a/mm/page_alloc.c~alloc_pages-permit-get_zeroed_pagegfp_atomic-from-interrupt-context
+++ a/mm/page_alloc.c
@@ -284,13 +284,25 @@ static inline void prep_zero_page(struct
 	int i;
 
 	VM_BUG_ON((gfp_flags & (__GFP_WAIT | __GFP_HIGHMEM)) == __GFP_HIGHMEM);
-	/*
-	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
-	 * and __GFP_HIGHMEM from hard or soft interrupt context.
-	 */
-	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
-	for (i = 0; i < (1 << order); i++)
-		clear_highpage(page + i);
+	if (gfp_flags & __GFP_HIGHMEM) {
+		/*
+		 * clear_highpage() will use KM_USER0, so it's a bug to use
+		 * __GFP_ZERO and __GFP_HIGHMEM from hard or soft interrupt
+		 * context.
+		 */
+		VM_BUG_ON(in_interrupt());
+		for (i = 0; i < (1 << order); i++)
+			clear_highpage(page + i);
+	} else {
+		/*
+		 * Go direct to clear_page(), because the caller might be
+		 * performing a non-highmem GFP_ZERO allocation from interrupt
+		 * context.  kmap_atomic() will go BUG when that happens, but it
+		 * is a legitimate thing to do
+		 */
+		for (i = 0; i < (1 << order); i++)
+			clear_page(page + i);
+	}
 }
 
 /*
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Thu, 23 Aug 2007, akpm@linux-foundation.org wrote:

> See http://bugzilla.kernel.org/show_bug.cgi?id=8928
> 
> I think it makes sense to permit a non-BUGging get_zeroed_page(GFP_ATOMIC)
> from interrupt context.

AFAIK this works now. GFP_ATOMIC does not set __GFP_HIGHMEM and thus the 
check

	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());

does not trigger

Any use of get_zeroed_page(  | __GFP_HIGHMEM) will cause a bug in

fastcall unsigned long get_zeroed_page(gfp_t gfp_mask)
{
        struct page * page;

        /*
         * get_zeroed_page() returns a 32-bit address, which cannot represent
         * a highmem page
         */
        VM_BUG_ON((gfp_mask & __GFP_HIGHMEM) != 0);

        page = alloc_pages(gfp_mask | __GFP_ZERO, 0);
        if (page)
                return (unsigned long) page_address(page);
        return 0;
}


And the patch does not change anything. We currently BUG_ON(GFP_HIGHMEM && 
in_interrupt) and after this patch we will still BUG(). The check was 
reordered but checks the same things. We could clear __GFP_HIGHMEM in 
__alloc_pages() if we are in an interrupt?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Andrew Morton]

On Mon, 27 Aug 2007 13:13:16 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> On Thu, 23 Aug 2007, akpm@linux-foundation.org wrote:
> 
> > See http://bugzilla.kernel.org/show_bug.cgi?id=8928
> > 
> > I think it makes sense to permit a non-BUGging get_zeroed_page(GFP_ATOMIC)
> > from interrupt context.
> 
> AFAIK this works now. GFP_ATOMIC does not set __GFP_HIGHMEM and thus the 
> check
> 
> 	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
> 
> does not trigger

The crash happens in

	clear_highpage
	->kmap_atomic
	  ->kmap_atomic_prot
	    ->BUG_ON(!pte_none(*(kmap_pte-idx)));

ie: this CPU held a kmap slot when the interrupt happened.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Mon, 27 Aug 2007, Andrew Morton wrote:

> > > I think it makes sense to permit a non-BUGging get_zeroed_page(GFP_ATOMIC)
> > > from interrupt context.
> > 
> > AFAIK this works now. GFP_ATOMIC does not set __GFP_HIGHMEM and thus the 
> > check
> > 
> > 	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
> > 
> > does not trigger
> 
> The crash happens in
> 
> 	clear_highpage
> 	->kmap_atomic
> 	  ->kmap_atomic_prot
> 	    ->BUG_ON(!pte_none(*(kmap_pte-idx)));
> 
> ie: this CPU held a kmap slot when the interrupt happened.

I guess I do not get what the problem is then. AFAIK: You cannot get there 
if you do a get_zeroed_page(GFP_ATOMIC). We should have bugged in 
get_zeroed_page() before we even got to clear_highpage.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Andrew Morton]

On Mon, 27 Aug 2007 14:00:04 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> On Mon, 27 Aug 2007, Andrew Morton wrote:
> 
> > > > I think it makes sense to permit a non-BUGging get_zeroed_page(GFP_ATOMIC)
> > > > from interrupt context.
> > > 
> > > AFAIK this works now. GFP_ATOMIC does not set __GFP_HIGHMEM and thus the 
> > > check
> > > 
> > > 	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
> > > 
> > > does not trigger
> > 
> > The crash happens in
> > 
> > 	clear_highpage
> > 	->kmap_atomic
> > 	  ->kmap_atomic_prot
> > 	    ->BUG_ON(!pte_none(*(kmap_pte-idx)));
> > 
> > ie: this CPU held a kmap slot when the interrupt happened.
> 
> I guess I do not get what the problem is then. AFAIK: You cannot get there 
> if you do a get_zeroed_page(GFP_ATOMIC). We should have bugged in 
> get_zeroed_page() before we even got to clear_highpage.
> 

: static inline void prep_zero_page(struct page *page, int order, gfp_t gfp_flags)
: {
: 	int i;
: 
: 	VM_BUG_ON((gfp_flags & (__GFP_WAIT | __GFP_HIGHMEM)) == __GFP_HIGHMEM);

__GFP_HIGHMEM is not set.

: 	/*
: 	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
: 	 * and __GFP_HIGHMEM from hard or soft interrupt context.
: 	 */
: 	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());

__GFP_HIGHMEM is not set

: 	for (i = 0; i < (1 << order); i++)
: 		clear_highpage(page + i);

kmap_atomic() goes boom.

: }


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Mon, 27 Aug 2007, Andrew Morton wrote:

> __GFP_HIGHMEM is not set.
> 
> : 	/*
> : 	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
> : 	 * and __GFP_HIGHMEM from hard or soft interrupt context.
> : 	 */
> : 	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
> 
> __GFP_HIGHMEM is not set
> 
> : 	for (i = 0; i < (1 << order); i++)
> : 		clear_highpage(page + i);
> 
> kmap_atomic() goes boom.

So the page is not a highmem page. kmap does:

void *kmap(struct page *page)
{
        might_sleep();
        if (!PageHighMem(page))
                return page_address(page);
        return kmap_high(page);
}

-> kmap is fine.

kmap_atomic() does:

void *kmap_atomic_prot(struct page *page, enum km_type type, pgprot_t prot)
{
        enum fixed_addresses idx;
        unsigned long vaddr;

        /* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
        pagefault_disable();

        idx = type + KM_TYPE_NR*smp_processor_id();
        BUG_ON(!pte_none(*(kmap_pte-idx)));

        if (!PageHighMem(page))
                return page_address(page);

        vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
        set_pte(kmap_pte-idx, mk_pte(page, prot));
        arch_flush_lazy_mmu_mode();

        return (void*) vaddr;
}

Move the check for highmem to the beginning of the function? Why 
should kmap_atomic fail for a non highmem page?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

This issue is a result of commit 656dad312fb41ed95ef08325e9df9bece3aacbbb. 
The intend of moving tests before the check for the highpage was to catch 
some additional errors.

On Mon, 27 Aug 2007, Christoph Lameter wrote:

> On Mon, 27 Aug 2007, Andrew Morton wrote:
> 
> > __GFP_HIGHMEM is not set.
> > 
> > : 	/*
> > : 	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
> > : 	 * and __GFP_HIGHMEM from hard or soft interrupt context.
> > : 	 */
> > : 	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
> > 
> > __GFP_HIGHMEM is not set
> > 
> > : 	for (i = 0; i < (1 << order); i++)
> > : 		clear_highpage(page + i);
> > 
> > kmap_atomic() goes boom.
> 
> So the page is not a highmem page. kmap does:
> 
> void *kmap(struct page *page)
> {
>         might_sleep();
>         if (!PageHighMem(page))
>                 return page_address(page);
>         return kmap_high(page);
> }
> 
> -> kmap is fine.
> 
> kmap_atomic() does:
> 
> void *kmap_atomic_prot(struct page *page, enum km_type type, pgprot_t prot)
> {
>         enum fixed_addresses idx;
>         unsigned long vaddr;
> 
>         /* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
>         pagefault_disable();
> 
>         idx = type + KM_TYPE_NR*smp_processor_id();
>         BUG_ON(!pte_none(*(kmap_pte-idx)));
> 
>         if (!PageHighMem(page))
>                 return page_address(page);
> 
>         vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
>         set_pte(kmap_pte-idx, mk_pte(page, prot));
>         arch_flush_lazy_mmu_mode();
> 
>         return (void*) vaddr;
> }
> 
> Move the check for highmem to the beginning of the function? Why 
> should kmap_atomic fail for a non highmem page?
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Andrew Morton]

On Mon, 27 Aug 2007 14:20:57 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> kmap_atomic() does:
> 
> void *kmap_atomic_prot(struct page *page, enum km_type type, pgprot_t prot)
> {
>         enum fixed_addresses idx;
>         unsigned long vaddr;
> 
>         /* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
>         pagefault_disable();
> 
>         idx = type + KM_TYPE_NR*smp_processor_id();
>         BUG_ON(!pte_none(*(kmap_pte-idx)));
> 
>         if (!PageHighMem(page))
>                 return page_address(page);
> 
>         vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
>         set_pte(kmap_pte-idx, mk_pte(page, prot));
>         arch_flush_lazy_mmu_mode();
> 
>         return (void*) vaddr;
> }
> 
> Move the check for highmem to the beginning of the function? Why 
> should kmap_atomic fail for a non highmem page?

For test coverage, mainly.  If someone is testing highmem-enabled code on
a 512MB machine, we want them to get told about any highmem-handling bugs,
even though they don't have highmem.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Mon, 27 Aug 2007, Andrew Morton wrote:

> > Move the check for highmem to the beginning of the function? Why 
> > should kmap_atomic fail for a non highmem page?
> 
> For test coverage, mainly.  If someone is testing highmem-enabled code on
> a 512MB machine, we want them to get told about any highmem-handling bugs,
> even though they don't have highmem.

But the test is for nested kmap_atomics. Nesting (at least allocate a 
regular page while highmem page is being mapped) needs to work in order to 
be able to allocate a page from an interrupt contexts.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Andrew Morton]

On Mon, 27 Aug 2007 14:43:46 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> On Mon, 27 Aug 2007, Andrew Morton wrote:
> 
> > > Move the check for highmem to the beginning of the function? Why 
> > > should kmap_atomic fail for a non highmem page?
> > 
> > For test coverage, mainly.  If someone is testing highmem-enabled code on
> > a 512MB machine, we want them to get told about any highmem-handling bugs,
> > even though they don't have highmem.
> 
> But the test is for nested kmap_atomics.

Sure.  If process-context code uses KM_USER0 and then an interrupt occurs
and interrupt-level code uses KM_USER0, Ingo's test will go boom.

> Nesting (at least allocate a 
> regular page while highmem page is being mapped) needs to work in order to 
> be able to allocate a page from an interrupt contexts.

yup, but interrupt-level code should use the reserved-for-interrupt kmap
slots (KM_IRQ0, etc).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Mon, 27 Aug 2007, Andrew Morton wrote:

> > Nesting (at least allocate a 
> > regular page while highmem page is being mapped) needs to work in order to 
> > be able to allocate a page from an interrupt contexts.
> 
> yup, but interrupt-level code should use the reserved-for-interrupt kmap
> slots (KM_IRQ0, etc).

We are not using any kmap slot since we are allocating a non highmem page!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Andrew Morton]

On Mon, 27 Aug 2007 15:12:58 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> On Mon, 27 Aug 2007, Andrew Morton wrote:
> 
> > > Nesting (at least allocate a 
> > > regular page while highmem page is being mapped) needs to work in order to 
> > > be able to allocate a page from an interrupt contexts.
> > 
> > yup, but interrupt-level code should use the reserved-for-interrupt kmap
> > slots (KM_IRQ0, etc).
> 
> We are not using any kmap slot since we are allocating a non highmem page!

Right.  So a get_zeroed_page() from IRQ context happens to be not buggy,
even though it goes BUG.

So what do we do?

a) don't call clear_highpage() for non-highmem pages (my fix)

b) don't do the is-the-pte-zero check in kmap_atomic_prot() if the page
   isn't highmem

   This is bad because it'll disable the check completely if the machine
   doesn't physically have highmem, or if the page happened to be a lowmem
   one.

c) don't do the is-the-pte-zero check if __GFP_HIGHMEM wasn't set.

   OK, but we don't pass the gfp_t into kmap_atomic.  So we need to do
   this at the caller site.  That's what a) does.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Mon, 27 Aug 2007, Andrew Morton wrote:

> So what do we do?
> 
> a) don't call clear_highpage() for non-highmem pages (my fix)

Then we need to be sure that this is the only case where we call 
clear_highpage() from an interrupt. This solves an i386 arch problem in 
core code. Other arches are not broken like that. See f.e. sparc:

void *kmap_atomic(struct page *page, enum km_type type)
{
        unsigned long idx;
        unsigned long vaddr;

        /* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
        pagefault_disable();
        if (!PageHighMem(page))
                return page_address(page);

        idx = type + KM_TYPE_NR*smp_processor_id();
        vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);


> b) don't do the is-the-pte-zero check in kmap_atomic_prot() if the page
>    isn't highmem
> 
>    This is bad because it'll disable the check completely if the machine
>    doesn't physically have highmem, or if the page happened to be a lowmem
>    one.

The check is meaningless if we do not have highmem. kmap_atomic is a 
function to be used in atomic context. I.e. interrupts. Nested by 
definition. It is broken as is since it BUG()s on a legitimate nested 
call.

> c) don't do the is-the-pte-zero check if __GFP_HIGHMEM wasn't set.
> 
>    OK, but we don't pass the gfp_t into kmap_atomic.  So we need to do
>    this at the caller site.  That's what a) does.

Would that not mean leaving kmap_atomic broken on i386? Before Ingo's 
commit things were fine. Revert the commit and there is no need 
to change core code.

What exactly is the point of checking that a kmap_atomic of a non highmem 
page cannot occur during the kmap_atomic of a highmem page? AFAICT this is 
fine.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Andrew Morton]

On Mon, 27 Aug 2007 16:01:28 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> On Mon, 27 Aug 2007, Andrew Morton wrote:
> 
> > So what do we do?
> > 
> > a) don't call clear_highpage() for non-highmem pages (my fix)
> 
> Then we need to be sure that this is the only case where we call 
> clear_highpage() from an interrupt. This solves an i386 arch problem in 
> core code. Other arches are not broken like that. See f.e. sparc:
> 
> void *kmap_atomic(struct page *page, enum km_type type)
> {
>         unsigned long idx;
>         unsigned long vaddr;
> 
>         /* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
>         pagefault_disable();
>         if (!PageHighMem(page))
>                 return page_address(page);
> 
>         idx = type + KM_TYPE_NR*smp_processor_id();
>         vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
> 
> 
> > b) don't do the is-the-pte-zero check in kmap_atomic_prot() if the page
> >    isn't highmem
> > 
> >    This is bad because it'll disable the check completely if the machine
> >    doesn't physically have highmem, or if the page happened to be a lowmem
> >    one.
> 
> The check is meaningless if we do not have highmem.

umm, yeah, it'll never trigger on a non-highmem machine.

> kmap_atomic is a 
> function to be used in atomic context. I.e. interrupts. Nested by 
> definition. It is broken as is since it BUG()s on a legitimate nested 
> call.

Is it broken?  Dunno.  It's a bit silly to run kmap_atomic() against a page
which the caller *knows* cannot be a highmem page.

The only situation where this is likely to occur is where the caller
received a gfp_t from higher up, like this case.

> > c) don't do the is-the-pte-zero check if __GFP_HIGHMEM wasn't set.
> > 
> >    OK, but we don't pass the gfp_t into kmap_atomic.  So we need to do
> >    this at the caller site.  That's what a) does.
> 
> Would that not mean leaving kmap_atomic broken on i386? Before Ingo's 
> commit things were fine. Revert the commit and there is no need 
> to change core code.

If we revert the commit we lose a bit of debug support.

We could move the assert to after we've checked for PageHighmem, but then
we'd fail to detect a bug if the nested caller happened to get a lowmem
page for a __GFP_HIGHMEM allocation.

> What exactly is the point of checking that a kmap_atomic of a non highmem 
> page cannot occur during the kmap_atomic of a highmem page? AFAICT this is 
> fine.

Just that the non-highmem page _might_ have been a highmem page, only the
caller got lucky.

I dunno, it's all a bit marginal, and tricky.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/1] alloc_pages(): permit get_zeroed_page(GFP_ATOMIC) from interrupt context

[Christoph Lameter]

On Mon, 27 Aug 2007, Andrew Morton wrote:

> > kmap_atomic is a 
> > function to be used in atomic context. I.e. interrupts. Nested by 
> > definition. It is broken as is since it BUG()s on a legitimate nested 
> > call.
> 
> Is it broken?  Dunno.  It's a bit silly to run kmap_atomic() against a page
> which the caller *knows* cannot be a highmem page.

So far we allow running kmap_atomic against a non-highmem page and 
kmap_atomic contains code to deal with that case.

> > Would that not mean leaving kmap_atomic broken on i386? Before Ingo's 
> > commit things were fine. Revert the commit and there is no need 
> > to change core code.
> 
> If we revert the commit we lose a bit of debug support.
> 
> We could move the assert to after we've checked for PageHighmem, but then
> we'd fail to detect a bug if the nested caller happened to get a lowmem
> page for a __GFP_HIGHMEM allocation.

We will ultimately detect it if he gets that type of page. Like many 
other checks in the code it may only trigger sometimes. Reverting 
656dad312fb41ed95ef08325e9df9bece3aacbbb will get us to a known good 
situation that also triggers the bug.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>