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

[Linus Torvalds <torvalds@linux-foundation.org>]

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