2012-06-18 13:10:44

by Wanpeng Li

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

From: Wanpeng Li <[email protected]>

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 <[email protected]>
---
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


2012-06-18 13:12:11

by Glauber Costa

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

On 06/18/2012 05:10 PM, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> 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.

2012-06-18 13:19:46

by Wanpeng Li

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

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 <[email protected]>
>>
>>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 [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-06-18 13:21:49

by Glauber Costa

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

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 <[email protected]>
>>>
>>> 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 ?

2012-06-19 00:05:25

by Kamezawa Hiroyuki

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

(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<[email protected]>
>>>
>>> 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