[PATCH] mm/memcg: add unlikely to mercg->move_charge_at_immigrate

[PATCH] mm/memcg: add unlikely to mercg->move_charge_at_immigrate

[Wanpeng Li]

From: Wanpeng Li <liwp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

move_charge_at_immigrate feature is disabled by default. Charges
are moved only when you move mm->owner and it also add additional
overhead.

Signed-off-by: Wanpeng Li <liwp.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 mm/memcontrol.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a9c3d01..795a00f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5316,7 +5316,7 @@ static int mem_cgroup_can_attach(struct cgroup *cgroup,
 	int ret = 0;
 	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
 
-	if (memcg->move_charge_at_immigrate) {
+	if (unlikely(memcg->move_charge_at_immigrate)) {
 		struct mm_struct *mm;
 		struct mem_cgroup *from = mem_cgroup_from_task(p);
 
-- 
1.7.9.5

Re: [PATCH] mm/memcg: add unlikely to mercg->move_charge_at_immigrate

[Glauber Costa]

On 06/18/2012 05:10 PM, Wanpeng Li wrote:
> From: Wanpeng Li <liwp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>
> move_charge_at_immigrate feature is disabled by default. Charges
> are moved only when you move mm->owner and it also add additional
> overhead.

How big is this overhead?

That's hardly a fast path. And if it happens to matter, it will be just 
bigger when you enable it, and the compiler start giving the wrong hints 
to the code.

Re: [PATCH] mm/memcg: add unlikely to mercg->move_charge_at_immigrate

[Wanpeng Li]

On Mon, Jun 18, 2012 at 05:09:36PM +0400, Glauber Costa wrote:
>On 06/18/2012 05:10 PM, Wanpeng Li wrote:
>>From: Wanpeng Li <liwp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>
>>move_charge_at_immigrate feature is disabled by default. Charges
>>are moved only when you move mm->owner and it also add additional
>>overhead.
>
>How big is this overhead?
>
>That's hardly a fast path. And if it happens to matter, it will be
>just bigger when you enable it, and the compiler start giving the
>wrong hints to the code.

Thank you for your quick response.

Oh, Maybe I should just write comments "move_charge_at_immigrate feature
is disabled by default. So add "unlikely", in order to compiler can optimize."

Best Regards,
Wanpeng Li

>--
>To unsubscribe from this list: send the line "unsubscribe cgroups" in
>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mm/memcg: add unlikely to mercg->move_charge_at_immigrate

[Glauber Costa]

On 06/18/2012 05:19 PM, Wanpeng Li wrote:
> On Mon, Jun 18, 2012 at 05:09:36PM +0400, Glauber Costa wrote:
>> On 06/18/2012 05:10 PM, Wanpeng Li wrote:
>>> From: Wanpeng Li <liwp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>>
>>> move_charge_at_immigrate feature is disabled by default. Charges
>>> are moved only when you move mm->owner and it also add additional
>>> overhead.
>>
>> How big is this overhead?
>>
>> That's hardly a fast path. And if it happens to matter, it will be
>> just bigger when you enable it, and the compiler start giving the
>> wrong hints to the code.
>
> Thank you for your quick response.
>
> Oh, Maybe I should just write comments "move_charge_at_immigrate feature
> is disabled by default. So add "unlikely", in order to compiler can optimize."
>

Again, do you have any reason to optimize that ?

Re: [PATCH] mm/memcg: add unlikely to mercg->move_charge_at_immigrate

[Kamezawa Hiroyuki]

(2012/06/18 22:19), Wanpeng Li wrote:
> On Mon, Jun 18, 2012 at 05:09:36PM +0400, Glauber Costa wrote:
>> On 06/18/2012 05:10 PM, Wanpeng Li wrote:
>>> From: Wanpeng Li<liwp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>>
>>> move_charge_at_immigrate feature is disabled by default. Charges
>>> are moved only when you move mm->owner and it also add additional
>>> overhead.
>>
>> How big is this overhead?
>>
>> That's hardly a fast path. And if it happens to matter, it will be
>> just bigger when you enable it, and the compiler start giving the
>> wrong hints to the code.
>
> Thank you for your quick response.
>
> Oh, Maybe I should just write comments "move_charge_at_immigrate feature
> is disabled by default. So add "unlikely", in order to compiler can optimize."
>

This doesn't affect the performance. likely/unlikely which doesn't affect
performance is never welcomed.

Thanks,
-Kame