From: Tejun Heo <tj@kernel.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: rjw@sisk.pl, paul@paulmenage.org, lizf@cn.fujitsu.com,
linux-pm@lists.linux-foundation.org,
linux-kernel@vger.kernel.org,
containers@lists.linux-foundation.org
Subject: Re: [PATCHSET] cgroup: introduce cgroup_taskset and consolidate subsys methods
Date: Wed, 24 Aug 2011 09:49:59 +0200 [thread overview]
Message-ID: <20110824074959.GA14170@htj.dyndns.org> (raw)
In-Reply-To: <20110824011428.GC23979@somewhere>
Hello, Frederic.
On Wed, Aug 24, 2011 at 03:14:30AM +0200, Frederic Weisbecker wrote:
> > 0001-cgroup-subsys-attach_task-should-be-called-after-mig.patch
> > 0002-cgroup-improve-old-cgroup-handling-in-cgroup_attach_.patch
> > 0003-cgroup-introduce-cgroup_taskset-and-use-it-in-subsys.patch
> > 0004-cgroup-don-t-use-subsys-can_attach_task-or-attach_ta.patch
> > 0005-cgroup-cpuset-don-t-use-ss-pre_attach.patch
> > 0006-cgroup-kill-subsys-can_attach_task-pre_attach-and-at.patch
>
> I don't understand the point on patches 3,4,5,6
>
> Why pushing the task iterations down to the subsystems?
I'll try again.
It seems like methods were added to serve the immediate need of the
particular user at the time and that in turn led to addition of
callbacks which were both superflous and incomplete (the bullet points
in the original message list them). This seems to have happened
because extra interface was added without trying to make the existing
interface complete.
The interface is complicated and cumbersome to use - are
[can_]attach() called first or [can_]attach_task()? What about
cancelation? What if a subsys wants to perform operations across
multiple tasks atomically?
In general, iteration-by-callback is painful to use. Establishing
common context (be it synchronization domain or shared variables)
becomes very cumbersome and implementation becomes fragmented and
difficult to follow. For example, imagine how it would be like to use
list if we had call_for_each_list_entry(func, list_head) instead of
the control-loop style iterators we have know.
So, using iterators enables making all relevant information to each
stage of attach so that only one callback is required for each step -
the way it should be. In addition, it makes it far easier for
subsystems to implement more involved logic in their methods.
I tried to make cgroup_freezer behave better which requires better
synchronization against the freezer and, with the current interface,
it's extremely ugly and painful. The new interface is complete, easy
to understand and use with far less subtleties.
Thanks.
--
tejun
next prev parent reply other threads:[~2011-08-24 7:50 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-23 22:19 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
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 [this message]
2011-08-24 13:53 ` 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=20110824074959.GA14170@htj.dyndns.org \
--to=tj@kernel.org \
--cc=containers@lists.linux-foundation.org \
--cc=fweisbec@gmail.com \
--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
all inboxes | Powered by JetHome®