The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Paul Menage <menage@google.com>, Li Zefan <lizf@cn.fujitsu.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Aditya Kali <adityakali@google.com>
Subject: Re: [PATCH 5/7] cgroups: Ability to stop res charge propagation on bounded ancestor
Date: Wed, 13 Jul 2011 15:50:42 +0200	[thread overview]
Message-ID: <20110713135039.GF9201@somewhere> (raw)
In-Reply-To: <20110712091131.e74d18f4.kamezawa.hiroyu@jp.fujitsu.com>

On Tue, Jul 12, 2011 at 09:11:31AM +0900, KAMEZAWA Hiroyuki wrote:
> On Mon, 11 Jul 2011 16:15:04 +0200
> Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > Moving a task from a cgroup to another may require to substract
> > its resource charge from the old cgroup and add it to the new one.
> > 
> > For this to happen, the uncharge/charge propagation can just stop
> > when we reach the common ancestor for the two cgroups. Further
> > the performance reasons, we also want to avoid to temporarily
> > overload the common ancestors with a non-accurate resource
> > counter usage if we charge first the new cgroup and uncharge the
> > old one thereafter. This is going to be a requirement for the coming
> > max number of task subsystem.
> > 
> > To solve this, provide a pair of new API that can charge/uncharge
> > a resource counter until we reach a given ancestor.
> > 
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Paul Menage <menage@google.com>
> > Cc: Li Zefan <lizf@cn.fujitsu.com>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Aditya Kali <adityakali@google.com>
> 
> 
> Hmm, do you have the number to show the benefit of this new function ?
> And....tasks is moving among cgroups so frequently as to show the benefit
> of this function in your environment ??

So the benefit is not really in the optimization, although that's a side effect.

Let me clarify the point in the changelog.
Imagine we have these cgroups:


                 A  (usage = 2, limit = 2)
                 |
                / \
               /   \
              /     \
             /       \
            /         \
           /           \
          B             C  (usage = 1, limit = 2)
(usage = 1, limit = 2)


The usage in A is the accumulation of the usage in B and C.
Imagine i want to move a task from C to B. This should work well.
We need to first check if we can charge B and do it, and then later
uncharge C.

But if we do:

	err = res_counter_charge(B)
	if (err)
		exit
	res_counter_uncharge(C)

it is going to fail because charging B will also charge A. And A
will refuse because it's already full. Ideally we should first uncharge
C and then charge B, so that A doesn't reject:

	res_counter_uncharge(C)
	err = res_countrer_charge(B)
	if (err)
		res_counter_charge(C)

The problem is that if charging B fails we need to rollback on C, but it might
be too late as a fork might have happen inside C since we uncharged it, so we couldn't
charge it back.

So the only solution is to first charge B but stop the charge propagation on A.
And then uncharge on C but stop uncharge on A.

  reply	other threads:[~2011-07-13 13:50 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-11 14:14 [PATCH 0/7] cgroups: New max number of tasks subsystem (was: cgroups rlim subsystem) Frederic Weisbecker
2011-07-11 14:15 ` [PATCH 1/7] cgroups: Add res_counter_write_u64() API Frederic Weisbecker
2011-07-11 20:30   ` Paul Menage
2011-07-11 14:15 ` [PATCH 2/7] cgroups: New resource counter inheritance API Frederic Weisbecker
2011-07-11 20:41   ` Paul Menage
2011-07-13 12:34     ` Frederic Weisbecker
2011-07-11 14:15 ` [PATCH 3/7] cgroups: Add previous cgroup in can_attach_task/attach_task callbacks Frederic Weisbecker
2011-07-11 20:42   ` Paul Menage
2011-07-11 14:15 ` [PATCH 4/7] cgroups: New cancel_attach_task subsystem callback Frederic Weisbecker
2011-07-26  0:57   ` Paul Menage
2011-07-11 14:15 ` [PATCH 5/7] cgroups: Ability to stop res charge propagation on bounded ancestor Frederic Weisbecker
2011-07-12  0:11   ` KAMEZAWA Hiroyuki
2011-07-13 13:50     ` Frederic Weisbecker [this message]
2011-07-26  0:50   ` Paul Menage
2011-07-11 14:15 ` [PATCH 6/7] cgroups: Add res counter common ancestor searching Frederic Weisbecker
2011-07-26  1:05   ` Paul Menage
2011-07-28 14:54     ` Frederic Weisbecker
2011-07-11 14:15 ` [PATCH 7/7] cgroups: Add a max number of tasks subsystem Frederic Weisbecker
2011-07-26  1:17   ` Paul Menage
2011-07-26  1:25     ` Li Zefan
2011-07-28 15:06     ` Frederic Weisbecker
2011-07-11 14:26 ` [PATCH 0/7] cgroups: New max number of tasks subsystem (was: cgroups rlim subsystem) Frederic Weisbecker

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=20110713135039.GF9201@somewhere \
    --to=fweisbec@gmail.com \
    --cc=adityakali@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=menage@google.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