All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhang Yanfei <zhangyanfei.yes@gmail.com>
To: David Rientjes <rientjes@google.com>
Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm: Add unlikely for current_order test
Date: Tue, 18 Jun 2013 05:56:47 +0800	[thread overview]
Message-ID: <51BF861F.6040802@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1306171431470.20631@chino.kir.corp.google.com>

On 06/18/2013 05:37 AM, David Rientjes wrote:
> On Mon, 17 Jun 2013, Zhang Yanfei wrote:
> 
>>> I don't understand the justification at all, current_order being unlikely 
>>> greater than or equal to pageblock_order / 2 doesn't imply at all that 
>>> it's unlikely that current_order is greater than or equal to 
>>> pageblock_order.
>>>
>>
>> hmmm... I am confused. Since current_order is >= pageblock_order / 2 is unlikely,
>> why current_order is >= pageblock_order isn't unlikely. Or there are other
>> tips?
>>
>> Actually, I am also a little confused about why current_order should be
>> unlikely greater than or equal to pageblock_order / 2. When borrowing pages
>> with other migrate_type, we always search from MAX_ORDER-1, which is greater
>> or equal to pageblock_order.
>>
> 
> Look at what is being done in the function: current_order loops down from 
> MAX_ORDER-1 to the order passed.  It is not at all "unlikely" that 
> current_order is greater than pageblock_order, or pageblock_order / 2.
> 
> MAX_ORDER is typically 11 and pageblock_order is typically 9 on x86.  
> Integer division truncates, so pageblock_order / 2 is 4.  For the first 
> eight iterations, it's guaranteed that current_order >= pageblock_order / 
> 2 if it even gets that far!
> 
> So just remove the unlikely() entirely, it's completely bogus.

I see. Thanks!

I will send another patch.

-- 
Thanks.
Zhang Yanfei

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

WARNING: multiple messages have this Message-ID (diff)
From: Zhang Yanfei <zhangyanfei.yes@gmail.com>
To: David Rientjes <rientjes@google.com>
Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm: Add unlikely for current_order test
Date: Tue, 18 Jun 2013 05:56:47 +0800	[thread overview]
Message-ID: <51BF861F.6040802@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1306171431470.20631@chino.kir.corp.google.com>

On 06/18/2013 05:37 AM, David Rientjes wrote:
> On Mon, 17 Jun 2013, Zhang Yanfei wrote:
> 
>>> I don't understand the justification at all, current_order being unlikely 
>>> greater than or equal to pageblock_order / 2 doesn't imply at all that 
>>> it's unlikely that current_order is greater than or equal to 
>>> pageblock_order.
>>>
>>
>> hmmm... I am confused. Since current_order is >= pageblock_order / 2 is unlikely,
>> why current_order is >= pageblock_order isn't unlikely. Or there are other
>> tips?
>>
>> Actually, I am also a little confused about why current_order should be
>> unlikely greater than or equal to pageblock_order / 2. When borrowing pages
>> with other migrate_type, we always search from MAX_ORDER-1, which is greater
>> or equal to pageblock_order.
>>
> 
> Look at what is being done in the function: current_order loops down from 
> MAX_ORDER-1 to the order passed.  It is not at all "unlikely" that 
> current_order is greater than pageblock_order, or pageblock_order / 2.
> 
> MAX_ORDER is typically 11 and pageblock_order is typically 9 on x86.  
> Integer division truncates, so pageblock_order / 2 is 4.  For the first 
> eight iterations, it's guaranteed that current_order >= pageblock_order / 
> 2 if it even gets that far!
> 
> So just remove the unlikely() entirely, it's completely bogus.

I see. Thanks!

I will send another patch.

-- 
Thanks.
Zhang Yanfei

  reply	other threads:[~2013-06-17 21:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-15 11:05 [PATCH] mm: Add unlikely for current_order test Zhang Yanfei
2013-06-15 11:05 ` Zhang Yanfei
2013-06-16 18:04 ` David Rientjes
2013-06-16 18:04   ` David Rientjes
2013-06-17  1:53   ` Zhang Yanfei
2013-06-17  1:53     ` Zhang Yanfei
2013-06-17 21:37     ` David Rientjes
2013-06-17 21:37       ` David Rientjes
2013-06-17 21:56       ` Zhang Yanfei [this message]
2013-06-17 21:56         ` Zhang Yanfei

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=51BF861F.6040802@gmail.com \
    --to=zhangyanfei.yes@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=zhangyanfei@cn.fujitsu.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.