linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Balbir Singh <bsingharora@gmail.com>,
	Ganesh Mahendran <opensource.ganesh@gmail.com>
Cc: linux-mm <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	mhocko@suse.com, mina86@mina86.com,
	Minchan Kim <minchan@kernel.org>,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>
Subject: Re: [PATCH v2] mm/page_alloc: remove unnecessary order check in __alloc_pages_direct_compact
Date: Wed, 15 Jun 2016 11:52:26 +0200	[thread overview]
Message-ID: <16e37ddf-c28d-23ea-1216-d5a9c8a81b58@suse.cz> (raw)
In-Reply-To: <CAKTCnzk1GZ+=ijvOm=Tw1GNGLdefovvS5wsR9XqpLLmrSSx9=g@mail.gmail.com>

On 06/15/2016 11:40 AM, Balbir Singh wrote:
> On Wed, Jun 15, 2016 at 7:34 PM, Ganesh Mahendran
> <opensource.ganesh@gmail.com> wrote:
>> In the callee try_to_compact_pages(), the (order == 0) is checked,
>> so remove check in __alloc_pages_direct_compact.
>>
>> Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
>> ---
>> v2:
>>   remove the check in __alloc_pages_direct_compact - Anshuman Khandual
>> ---
>>  mm/page_alloc.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index b9ea618..2f5a82a 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -3173,9 +3173,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>>         struct page *page;
>>         int contended_compaction;
>>
>> -       if (!order)
>> -               return NULL;
>> -
>>         current->flags |= PF_MEMALLOC;
>>         *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
>>                                                 mode, &contended_compaction);
>
> What is the benefit of this. Is an if check more expensive than
> calling the function and returning from it? I don't feel strongly
> about such changes, but its good to audit the overall code for reading
> and performance.

Agree. The majority of calls should be for order == 0 where the check 
avoids us from modifying current->flags and calling into compaction.c 
just to return and modify the flags back. I would argue that we should 
even check order before calling __alloc_pages_direct_compact() to avoid 
another potential call, but the compiler might be doing the right thing 
already.

So v1 was better in this aspect. But it wouldn't gain us any measurable 
performance benefit anyway, so we might as well leave it.

> Balbir Singh
>

--
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:[~2016-06-15  9:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-15  9:34 Ganesh Mahendran
2016-06-15  9:40 ` Balbir Singh
2016-06-15  9:52   ` Vlastimil Babka [this message]
2016-06-15 16:41   ` Michal Nazarewicz

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=16e37ddf-c28d-23ea-1216-d5a9c8a81b58@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=bsingharora@gmail.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=khandual@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=mina86@mina86.com \
    --cc=minchan@kernel.org \
    --cc=opensource.ganesh@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