linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: mgorman@techsingularity.net (Mel Gorman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
Date: Mon, 19 Oct 2015 14:26:49 +0100	[thread overview]
Message-ID: <20151019132649.GA2629@techsingularity.net> (raw)
In-Reply-To: <5624E561.6010107@arm.com>

On Mon, Oct 19, 2015 at 01:43:13PM +0100, Robin Murphy wrote:
> Hi Andrew,
> 
> On 16/10/15 21:59, Andrew Morton wrote:
> >On Fri, 16 Oct 2015 16:33:42 +0100 Robin Murphy <robin.murphy@arm.com> wrote:
> >
> >>__GFP_WAIT is going away to live its life under a new identity; convert
> >>__iommu_alloc_attrs() to the new helper function instead.
> >>
> >>...
> >>
> >>--- a/arch/arm64/mm/dma-mapping.c
> >>+++ b/arch/arm64/mm/dma-mapping.c
> >>@@ -566,7 +566,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
> >>  	 */
> >>  	gfp |= __GFP_ZERO;
> >>
> >>-	if (gfp & __GFP_WAIT) {
> >>+	if (gfpflags_allow_blocking(gfp)) {
> >>  		struct page **pages;
> >>  		pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> >
> >Seems unnecessarily elaborate.  What's wrong with
> >
> >--- a/arch/arm64/mm/dma-mapping.c~mm-page_alloc-rename-__gfp_wait-to-__gfp_reclaim-arm-fix
> >+++ a/arch/arm64/mm/dma-mapping.c
> >@@ -562,7 +562,7 @@ static void *__iommu_alloc_attrs(struct
> >  	 */
> >  	gfp |= __GFP_ZERO;
> >
> >-	if (gfp & __GFP_WAIT) {
> >+	if (gfp & __GFP_RECLAIM) {
> >  		struct page **pages;
> >  		pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> >
> >
> >?
> 
> Well, in that case the charge of "unnecessarily elaborate" should have been
> directed at the original patch, and the 53 other locations where (flags &
> __GFP_WAIT) was changed as per the commit message:
> 
>   "Callers that are checking if they are non-blocking should use the
>    helper gfpflags_allow_blocking() where possible."
> 

The use of gfpflags_allows_blocking() like you originally had is actually
preferred by me. __GFP_RECLAIM can return true when the caller only allows
kswapd to wake which has nothing to do with blocking (currently). The
helper was added to avoid this type of confusion.

-- 
Mel Gorman
SUSE Labs

  reply	other threads:[~2015-10-19 13:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16 15:33 [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Robin Murphy
2015-10-16 15:33 ` [PATCH 2/2] arm64: Use gfpflags_allow_blocking() Robin Murphy
2015-10-16 16:20   ` Catalin Marinas
2015-10-16 20:59   ` Andrew Morton
2015-10-19 12:43     ` Robin Murphy
2015-10-19 13:26       ` Mel Gorman [this message]
2015-10-16 16:20 ` [PATCH 1/2] arm64: Workaround renaming of __GFP_WAIT Catalin Marinas
2015-10-28  0:53 ` Joerg Roedel
2015-10-28 11:01   ` Robin Murphy

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=20151019132649.GA2629@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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).