linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: MinChan Kim <minchan.kim@gmail.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mmtom :  add VM_BUG_ON in __get_free_pages
Date: Mon, 2 Mar 2009 14:27:57 -0800	[thread overview]
Message-ID: <20090302142757.1cc014aa.akpm@linux-foundation.org> (raw)
In-Reply-To: <20090302183148.a4dfcc22.minchan.kim@barrios-desktop>

On Mon, 2 Mar 2009 18:31:48 +0900
MinChan Kim <minchan.kim@gmail.com> wrote:

> 
> The __get_free_pages is used in many place. 
> Also, driver developers can use it freely due to export function.
> Some developers might use it to allocate high pages by mistake. 
> 
> The __get_free_pages can allocate high page using alloc_pages, 
> but it can't return linear address for high page.
> 
> Even worse, in this csse, caller can't free page which are there in high zone. 
> So, It would be better to add VM_BUG_ON. 
> 
> It's based on mmtom 2009-02-27-13-54.
>  
> Signed-off-by: MinChan Kim <minchan.kim@gmail.com>
> ---
>  mm/page_alloc.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 8294107..381056b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1681,6 +1681,13 @@ EXPORT_SYMBOL(__alloc_pages_internal);
>  unsigned long __get_free_pages(gfp_t gfp_mask, unsigned int order)
>  {
>  	struct page * page;
> +
> +	/*
> +	 * __get_free_pages() returns a 32-bit address, which cannot represent
> +	 * a highmem page
> +	 */
> +	VM_BUG_ON((gfp_mask & __GFP_HIGHMEM) != 0);
> +
>  	page = alloc_pages(gfp_mask, order);
>  	if (!page)
>  		return 0;

If someone calls __get_free_pages(__GFP_HIGHMEM) then page_address()
will reliably return NULL and the caller's code will oops.

Yes, there's a decent (and increasing) risk that the developer won't be
testing the code on a highmem machine, but there are enough highmem
machines out there that the bug should be discovered pretty quickly.

So I'm not sure that this test is worth the additional overhead to a
fairly frequently called function?

--
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>

  reply	other threads:[~2009-03-02 22:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-02  9:31 MinChan Kim
2009-03-02 22:27 ` Andrew Morton [this message]
2009-03-02 23:01   ` Minchan Kim
2009-03-02 23:16   ` Johannes Weiner
2009-03-03 12:05     ` Minchan Kim

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=20090302142757.1cc014aa.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan.kim@gmail.com \
    /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