From: Mel Gorman <mgorman@techsingularity.net>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@kernel.org>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Takashi Iwai <tiwai@suse.de>, Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH v3] mm, page_alloc: disallow __GFP_COMP in alloc_pages_exact()
Date: Tue, 19 Mar 2019 09:47:33 +0000 [thread overview]
Message-ID: <20190319094733.4j6sp63ma56vygzr@techsingularity.net> (raw)
In-Reply-To: <0c6393eb-b28d-4607-c386-862a71f09de6@suse.cz>
On Mon, Mar 18, 2019 at 01:21:59PM +0100, Vlastimil Babka wrote:
> OK here's a new version that changes the patch to remove __GFP_COMP per
> the v2 discussion, and also fixes the bug Kirill spotted (thanks!).
>
> ----8<----
> From 1fbc84c208573b885f51818ed823f89b3aa1e0ae Mon Sep 17 00:00:00 2001
> From: Vlastimil Babka <vbabka@suse.cz>
> Date: Thu, 14 Mar 2019 10:19:30 +0100
> Subject: [PATCH v3] mm, page_alloc: disallow __GFP_COMP in alloc_pages_exact()
>
> alloc_pages_exact*() allocates a page of sufficient order and then splits it
> to return only the number of pages requested. That makes it incompatible with
> __GFP_COMP, because compound pages cannot be split.
>
> As shown by [1] things may silently work until the requested size (possibly
> depending on user) stops being power of two. Then for CONFIG_DEBUG_VM, BUG_ON()
> triggers in split_page(). Without CONFIG_DEBUG_VM, consequences are unclear.
>
> There are several options here, none of them great:
>
> 1) Don't do the spliting when __GFP_COMP is passed, and return the whole
> compound page. However if caller then returns it via free_pages_exact(),
> that will be unexpected and the freeing actions there will be wrong.
>
> 2) Warn and remove __GFP_COMP from the flags. But the caller may have really
> wanted it, so things may break later somewhere.
>
> 3) Warn and return NULL. However NULL may be unexpected, especially for
> small sizes.
>
> This patch picks option 2, because as Michal Hocko put it: "callers wanted it"
> is much less probable than "caller is simply confused and more gfp flags is
> surely better than fewer".
>
> [1] https://lore.kernel.org/lkml/20181126002805.GI18977@shao2-debian/T/#u
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Mel Gorman <mgorman@techsingularity.net>
prev parent reply other threads:[~2019-03-19 9:47 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-14 9:39 [PATCH] " Vlastimil Babka
2019-03-14 9:42 ` [PATCH v2] " Vlastimil Babka
2019-03-14 10:15 ` Michal Hocko
2019-03-14 10:30 ` Vlastimil Babka
2019-03-14 11:36 ` Michal Hocko
2019-03-14 11:56 ` Takashi Iwai
2019-03-14 12:09 ` Michal Hocko
2019-03-14 13:15 ` Takashi Iwai
2019-03-14 13:29 ` Michal Hocko
2019-03-14 16:52 ` Takashi Iwai
2019-03-14 17:37 ` Hugh Dickins
2019-03-14 18:00 ` Takashi Iwai
2019-03-14 18:15 ` Hugh Dickins
2019-03-14 20:13 ` Takashi Iwai
2019-03-14 18:51 ` Kirill A. Shutemov
2019-03-18 12:21 ` [PATCH v3] " Vlastimil Babka
2019-03-18 12:43 ` Michal Hocko
2019-03-19 8:45 ` Kirill A. Shutemov
2019-03-19 9:47 ` Mel Gorman [this message]
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=20190319094733.4j6sp63ma56vygzr@techsingularity.net \
--to=mgorman@techsingularity.net \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=tiwai@suse.de \
--cc=vbabka@suse.cz \
/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