linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: Zwane Mwaikambo <zwane@arm.linux.org.uk>
Cc: chrisw@osdl.org, clameter@sgi.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fixes for prep_zero_page
Date: Sun, 9 Jan 2005 12:52:12 -0800	[thread overview]
Message-ID: <20050109125212.330c34c1.akpm@osdl.org> (raw)
In-Reply-To: <Pine.LNX.4.61.0501090812220.13639@montezuma.fsmlabs.com>

Zwane Mwaikambo <zwane@arm.linux.org.uk> wrote:
>
> On Sun, 9 Jan 2005, Andrew Morton wrote:
> 
> > Well it's doing clear_highpage() before __alloc_pages() has called
> > kernel_map_pages(), so CONFIG_DEBUG_PAGEALLOC is quite kaput.
> > 
> > So the current __GFP_ZERO buglist is:
> > 
> > 1: Breaks CONFIG_DEBUG_PAGEALLOC
> > 
> > 2: Breaks the cache aliasing protection for anonymous pages
> > 
> > 3: prep_zero_page() uses KM_USER0 so __GFP_ZERO from IRQ context will
> >    cause rare memory corruption.
> 
> The following should take care of 1 and 3. I opted to unmap the pages 
> again after the clear page so that it remains isolated and we don't have 
> to make additional checks to see if we should unmap the pages.
> 
> Signed-off-by: Zwane Mwaikambo <zwane@arm.linux.org.uk>
> 
> Index: linux-2.6.10-mm2/include/linux/highmem.h
> ===================================================================
> RCS file: /home/cvsroot/linux-2.6.10-mm2/include/linux/highmem.h,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 highmem.h
> --- linux-2.6.10-mm2/include/linux/highmem.h	9 Jan 2005 04:51:52 -0000	1.1.1.1
> +++ linux-2.6.10-mm2/include/linux/highmem.h	9 Jan 2005 15:32:17 -0000
> @@ -50,6 +50,18 @@ static inline void clear_highpage(struct
>  	kunmap_atomic(kaddr, KM_USER0);
>  }
>  
> +static inline void clear_irq_highpage(struct page *page)
> +{
> +	char *kaddr;
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	kaddr = kmap_atomic(page, KM_IRQ0);
> +	clear_page(kaddr);
> +	kunmap_atomic(kaddr, KM_IRQ0);
> +	local_irq_restore(flags);
> +}
> +

This won't work right if someone tries to allocate memory while holding a
KM_IRQ0 atomic kmap.

It would be quite bizarre for anyone to be allocating highmem pages from
IRQ context anyway, but as a generic mechanism this really should work as
expected in all contexts.  That means a new kmap slot.

>  /*
>   * Same but also flushes aliased cache contents to RAM.
>   */
> Index: linux-2.6.10-mm2/mm/page_alloc.c
> ===================================================================
> RCS file: /home/cvsroot/linux-2.6.10-mm2/mm/page_alloc.c,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 page_alloc.c
> --- linux-2.6.10-mm2/mm/page_alloc.c	9 Jan 2005 04:52:40 -0000	1.1.1.1
> +++ linux-2.6.10-mm2/mm/page_alloc.c	9 Jan 2005 15:46:00 -0000
> @@ -691,10 +691,17 @@ perthread_pages_alloc(void)
>   */
>  static inline void prep_zero_page(struct page *page, int order)
>  {
> -	int i;
> +	int i, nr_pages = (1 << order);
> +	int context = in_interrupt();
>  
> -	for(i = 0; i < (1 << order); i++)
> -		clear_highpage(page + i);
> +	kernel_map_pages(page, nr_pages, 1);
> +	for(i = 0; i < nr_pages; i++) {
> +		if (likely(!context))
> +			clear_highpage(page + i);
> +		else
> +			clear_irq_highpage(page + i);
> +	}
> +	kernel_map_pages(page, nr_pages, 0);
>  }

Can't we simply move the page zeroing to the very end of __alloc_pages()?


  reply	other threads:[~2005-01-09 20:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-01-08  9:06 panic on bootup due to __GFP_ZERO patch Chris Wright
2005-01-08 20:00 ` Dave Jones
2005-01-09  9:45 ` Andrew Morton
2005-01-09 15:56   ` [PATCH] Fixes for prep_zero_page Zwane Mwaikambo
2005-01-09 20:52     ` Andrew Morton [this message]
2005-01-09 21:32       ` Zwane Mwaikambo
2005-01-09 22:48         ` Chris Wright
2005-01-10  4:18           ` Zwane Mwaikambo
2005-01-13  5:05             ` Zwane Mwaikambo
2005-01-13 18:24               ` Chris Wright
2005-01-14  0:50                 ` Zwane Mwaikambo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20050109125212.330c34c1.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=chrisw@osdl.org \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zwane@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).