All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Michal Hocko <mhocko@kernel.org>
Cc: JianKang Chen <chenjiankang1@huawei.com>,
	mgorman@techsingularity.net, hannes@cmpxchg.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	xieyisheng1@huawei.com, guohanjun@huawei.com,
	wangkefeng.wang@huawei.com
Subject: Re: [PATCH resend] mm/page_alloc: fix comment is __get_free_pages
Date: Wed, 29 Nov 2017 13:41:59 -0800	[thread overview]
Message-ID: <20171129134159.c9100ea6dacad870d69929b7@linux-foundation.org> (raw)
In-Reply-To: <20171129160446.jluzpv3n6mjc3fwv@dhcp22.suse.cz>

On Wed, 29 Nov 2017 17:04:46 +0100 Michal Hocko <mhocko@kernel.org> wrote:

> On Mon 27-11-17 12:33:41, Michal Hocko wrote:
> > On Mon 27-11-17 19:09:24, JianKang Chen wrote:
> > > From: Jiankang Chen <chenjiankang1@huawei.com>
> > > 
> > > __get_free_pages will return an virtual address, 
> > > but it is not just 32-bit address, for example a 64-bit system. 
> > > And this comment really confuse new bigenner of mm.
> > 
> > s@bigenner@beginner@
> > 
> > Anyway, do we really need a bug on for this? Has this actually caught
> > any wrong usage? VM_BUG_ON tends to be enabled these days AFAIK and
> > panicking the kernel seems like an over-reaction. If there is a real
> > risk then why don't we simply mask __GFP_HIGHMEM off when calling
> > alloc_pages?
> 
> I meant this
> ---
> >From 000bb422fe07adbfa8cd8ed953b18f48647a45d6 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 29 Nov 2017 17:02:33 +0100
> Subject: [PATCH] mm: drop VM_BUG_ON from __get_free_pages
> 
> There is no real reason to blow up just because the caller doesn't know
> that __get_free_pages cannot return highmem pages. Simply fix that up
> silently. Even if we have some confused users such a fixup will not be
> harmful.

mm...  So we have a caller which hopes to be getting highmem pages but
isn't.  Caller then proceeds to pointlessly kmap the page and wonders
why it isn't getting as much memory as it would like on 32-bit systems,
etc.

I do think we should help ferret out such bogosity.  A WARN_ON_ONCE
would suffice.

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Michal Hocko <mhocko@kernel.org>
Cc: JianKang Chen <chenjiankang1@huawei.com>,
	mgorman@techsingularity.net, hannes@cmpxchg.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	xieyisheng1@huawei.com, guohanjun@huawei.com,
	wangkefeng.wang@huawei.com
Subject: Re: [PATCH resend] mm/page_alloc: fix comment is __get_free_pages
Date: Wed, 29 Nov 2017 13:41:59 -0800	[thread overview]
Message-ID: <20171129134159.c9100ea6dacad870d69929b7@linux-foundation.org> (raw)
In-Reply-To: <20171129160446.jluzpv3n6mjc3fwv@dhcp22.suse.cz>

On Wed, 29 Nov 2017 17:04:46 +0100 Michal Hocko <mhocko@kernel.org> wrote:

> On Mon 27-11-17 12:33:41, Michal Hocko wrote:
> > On Mon 27-11-17 19:09:24, JianKang Chen wrote:
> > > From: Jiankang Chen <chenjiankang1@huawei.com>
> > > 
> > > __get_free_pages will return an virtual address, 
> > > but it is not just 32-bit address, for example a 64-bit system. 
> > > And this comment really confuse new bigenner of mm.
> > 
> > s@bigenner@beginner@
> > 
> > Anyway, do we really need a bug on for this? Has this actually caught
> > any wrong usage? VM_BUG_ON tends to be enabled these days AFAIK and
> > panicking the kernel seems like an over-reaction. If there is a real
> > risk then why don't we simply mask __GFP_HIGHMEM off when calling
> > alloc_pages?
> 
> I meant this
> ---
> >From 000bb422fe07adbfa8cd8ed953b18f48647a45d6 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 29 Nov 2017 17:02:33 +0100
> Subject: [PATCH] mm: drop VM_BUG_ON from __get_free_pages
> 
> There is no real reason to blow up just because the caller doesn't know
> that __get_free_pages cannot return highmem pages. Simply fix that up
> silently. Even if we have some confused users such a fixup will not be
> harmful.

mm...  So we have a caller which hopes to be getting highmem pages but
isn't.  Caller then proceeds to pointlessly kmap the page and wonders
why it isn't getting as much memory as it would like on 32-bit systems,
etc.

I do think we should help ferret out such bogosity.  A WARN_ON_ONCE
would suffice.

--
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:[~2017-11-29 21:42 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-27 11:09 [PATCH resend] mm/page_alloc: fix comment is __get_free_pages JianKang Chen
2017-11-27 11:09 ` JianKang Chen
2017-11-27 11:33 ` Michal Hocko
2017-11-27 11:33   ` Michal Hocko
2017-11-29 16:04   ` Michal Hocko
2017-11-29 16:04     ` Michal Hocko
2017-11-29 21:41     ` Andrew Morton [this message]
2017-11-29 21:41       ` Andrew Morton
2017-11-30  6:53       ` Michal Hocko
2017-11-30  6:53         ` Michal Hocko
2017-11-30 21:17         ` Andrew Morton
2017-11-30 21:17           ` Andrew Morton
2017-12-01  7:24           ` Michal Hocko
2017-12-01  7:24             ` Michal Hocko
2017-12-01 11:18             ` Michal Hocko
2017-12-01 11:18               ` Michal Hocko
2017-12-14 14:06               ` Michal Hocko
2017-12-14 14:06                 ` Michal Hocko
2017-12-14 20:33                 ` Andrew Morton
2017-12-14 20:33                   ` Andrew Morton
2017-12-15  9:36                   ` Michal Hocko
2017-12-15  9:36                     ` Michal Hocko
2017-12-15 20:57                     ` Andrew Morton
2017-12-15 20:57                       ` Andrew Morton
2017-12-16 11:52                       ` Michal Hocko
2017-12-16 11:52                         ` Michal Hocko
2018-04-06 10:02     ` Michal Hocko
2018-04-06 19:58       ` Andrew Morton
  -- strict thread matches above, loose matches on Subject: below --
2017-11-25  7:20 JianKang Chen
2017-11-25  7:20 ` JianKang Chen
2017-11-27  6:22 ` Anshuman Khandual
2017-11-27  6:22   ` Anshuman Khandual

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=20171129134159.c9100ea6dacad870d69929b7@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=chenjiankang1@huawei.com \
    --cc=guohanjun@huawei.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=wangkefeng.wang@huawei.com \
    --cc=xieyisheng1@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.