public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Paul Menage <paul@paulmenage.org>
Cc: rjw@sisk.pl, lizf@cn.fujitsu.com,
	linux-pm@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org,
	containers@lists.linux-foundation.org
Subject: Re: [PATCH 2/6] cgroup: improve old cgroup handling in cgroup_attach_proc()
Date: Thu, 25 Aug 2011 11:03:23 +0200	[thread overview]
Message-ID: <20110825090323.GD3286@htj.dyndns.org> (raw)
In-Reply-To: <CALdu-PAj1ZUmB2ixxA6yeppB8MerBGk1cSeQadobH0H4cRSe7Q@mail.gmail.com>

Hello,

On Thu, Aug 25, 2011 at 01:51:39AM -0700, Paul Menage wrote:
> In particular, making the group flex-array entries contain both a task
> and a cgroup appears to be only necessary in order to skip tasks where
> new_cgroup == old_cgroup. Can't we get the same effect by simply
> leaving all such tasks out of the flex-array in the first place?

In general, the interface *should* give full information to subsys
methods at each stage including the old cgroup each task is migrating
from and the new cgroup; otherwise, they soon end up developing weird
acrobatics to work around shortcomings in the interface or being
outright buggy, so let's please look past the fixes which are
necessary immediately and think about what a proper interface should
look like.

I mean, seriously, why did ->attach_task() take @new_cgroup when it's
called after migration happened while ->attach() had access to the old
cgroup of the last iterated task in the group?  What the hell does
that even mean?

And, why is this a big change?  The big part is change of interface
but that we need to do anyway.  This one is just adding an entry to
the flex array.

Thanks.

-- 
tejun

  reply	other threads:[~2011-08-25  9:03 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-23 22:19 [PATCHSET] cgroup: introduce cgroup_taskset and consolidate subsys methods Tejun Heo
2011-08-23 22:19 ` [PATCH 1/6] cgroup: subsys->attach_task() should be called after migration Tejun Heo
2011-08-24  0:32   ` Frederic Weisbecker
2011-08-24  1:31     ` Li Zefan
2011-08-23 22:19 ` [PATCH 2/6] cgroup: improve old cgroup handling in cgroup_attach_proc() Tejun Heo
2011-08-25  8:51   ` Paul Menage
2011-08-25  9:03     ` Tejun Heo [this message]
2011-08-25  9:42   ` Paul Menage
2011-08-25  9:44     ` Tejun Heo
2011-08-23 22:19 ` [PATCH 3/6] cgroup: introduce cgroup_taskset and use it in subsys->can_attach(), cancel_attach() and attach() Tejun Heo
2011-08-25  0:39   ` KAMEZAWA Hiroyuki
2011-08-25  8:20     ` Tejun Heo
2011-08-25  8:21       ` KAMEZAWA Hiroyuki
2011-08-25  8:40         ` Tejun Heo
2011-08-25  8:37           ` KAMEZAWA Hiroyuki
2011-08-25  9:14   ` Paul Menage
2011-08-25  9:20     ` Tejun Heo
2011-08-25  9:32       ` Paul Menage
2011-08-25  9:32   ` Paul Menage
2011-08-23 22:19 ` [PATCH 4/6] cgroup: don't use subsys->can_attach_task() or ->attach_task() Tejun Heo
2011-08-24  1:57   ` Matt Helsley
2011-08-24  7:54     ` Tejun Heo
2011-08-25  9:07   ` Paul Menage
2011-08-25  9:12     ` Tejun Heo
2011-08-23 22:19 ` [PATCH 5/6] cgroup, cpuset: don't use ss->pre_attach() Tejun Heo
2011-08-25  8:53   ` Paul Menage
2011-08-25  9:06     ` Tejun Heo
2011-08-23 22:20 ` [PATCH 6/6] cgroup: kill subsys->can_attach_task(), pre_attach() and attach_task() Tejun Heo
2011-08-25  9:45   ` Paul Menage
2011-08-24  1:14 ` [PATCHSET] cgroup: introduce cgroup_taskset and consolidate subsys methods Frederic Weisbecker
2011-08-24  7:49   ` Tejun Heo
2011-08-24 13:53     ` Frederic Weisbecker
  -- strict thread matches above, loose matches on Subject: below --
2011-08-25 22:43 [PATCHSET] cgroup: introduce cgroup_taskset and consolidate subsys methods, take#2 Tejun Heo
2011-08-25 22:43 ` [PATCH 2/6] cgroup: improve old cgroup handling in cgroup_attach_proc() Tejun Heo
2011-08-26  4:13   ` KAMEZAWA Hiroyuki

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=20110825090323.GD3286@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=paul@paulmenage.org \
    --cc=rjw@sisk.pl \
    /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