Re: 2.6.25-rc7-git2: Reported regressions from 2.6.24

From: Linus Torvalds
Date: Fri Mar 28 2008 - 15:27:17 EST




On Fri, 28 Mar 2008, Christoph Lameter wrote:
>
> prep_zero_page does:
>
> static inline void prep_zero_page(struct page *page, int order, gfp_t gfp_flags)
> {
> int i;
>
> /*
> * 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);

.. and clear_highpage() does:

void *kaddr = kmap_atomic(page, KM_USER0);
clear_page(kaddr);

.. where kmap_atomic() on x86 does:

kmap_atomic() ->
kmap_atomic_prot() ->
debug_kmap_atomic_prot() ->
if (in_irq())
WARN_ON_ONCE()

none of which are at all conditional on __GFP_HIGHMEM.

But none of this is relevant. The warning possibly didn't even come from
slub, it just made me look at it - because *something* is doing GFP_ATOMIC
together with __GFP_ZERO, and it became obvious that SLUB is one potential
cause of that.

And the SLUB case simply isn't valid!

> Then clear_highpage calls additional checking functions that have
> the effect of generally forbiding zeroing in interrupt context if
> CONFIG_HIGHMEM is set. This is wrong and needs to be fixed.

No. Dammit, the bug is in SLUB.

If SLUB *ever* calls the page allocator with __GFP_ZERO set, it's a
bug, and that has nothing to do with GFP_ATOMIC or anything else. Because
SLUB uses its own logic for clearing the result.

Why cannot you just admit it?

Now, _outside_ of SLUB there appear to be other users too, and those users
need to either be fixed or we need to allow __GFP_ZERO togethe with
GFP_ATOMIC. But the fact is, SLUB had a really stupid bug that it
shouldn't have had.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/