Messages in this thread | | | From | Paul Menage <> | Date | Fri, 26 Aug 2011 08:54:42 -0700 | Subject | Re: [PATCH 3/6] cgroup: introduce cgroup_taskset and use it in subsys->can_attach(), cancel_attach() and attach() |
| |
On Thu, Aug 25, 2011 at 7:38 PM, Matt Helsley <matthltc@us.ibm.com> wrote: > On Fri, Aug 26, 2011 at 12:43:09AM +0200, Tejun Heo wrote: >> Currently, there's no way to pass multiple tasks to cgroup_subsys >> methods necessitating the need for separate per-process and per-task >> methods. This patch introduces cgroup_taskset which can be used to >> pass multiple tasks and their associated cgroups to cgroup_subsys >> methods. > > This will be the third iterator-ish pattern in the cgroup code. > It's not your fault but it does seem a bit much to have:
I agree with this sentiment in principle (in fact it was one of the first things that I thought when I saw this patch) but I think that merging them is a non-trivial process, and so shouldn't hold up the introduction of this patch set.
Changing the cgroup_scanner interface to be control-loop rather than callback (for the processed tasks, at least - testing tasks will have to be a callback still, I think) shouldn't be too hard.
The important differences between the iterators then are how they generate a stable set of tasks. If we made the basic callback operations be:
start - do any prep work fill - return some tasks to process, or none if the iterator is exhausted end - undo any prep work
then the user API functions would look something like:
iter_start(iter *i) { i->start(); i->avail_task_count = i->avail_task_pos = 0; }
struct task_and_cgroup *iter_next(iter *i) { if (i->avail_task_pos >= i->avail_task_count) { i->avail_task_pos = 0; i->fill(&i->avail_tasks, &i->avail_task_count); if (!i->avail_tasks) return NULL; } } return i->avail_tasks + i->avail_task_pos++; }
iter_end(iter *i) { i->end(); }
then for the three iteration abstractions:
- cgroup_iter_start/next/end:
creation API: taskset_from_cgroup(cgroup *cg)
start - takes the lock fill - returns the next task in the cgroup, or NULL if exhausted end - releases the lock
- cgroup_taskset
creation API: taskset_from_array(task_and_cgroup *tg, int count)
start - no-op fill - returns a pointer to the array used to set up the taskset end - no-op
- cgroup_scanner
creation API: atomic_taskset_from_array(cgroup, test_fn, optional_heap);
start - allocate an array for storing the heap used to build the next batch of tasks (if not provided by creator) fill - takes css_set lock and makes a pass over the cgroup to find the lowest (by start-time) set of unprocessed tasks that match the test_fn predicate, releases css_set_lock end - frees the array (if not provided by the creator)
I think the progression of patches would be:
- convert the current user of cgroup_scanner to a control-loop API rather than a callback API (for process_task) just to verify that this fits in the existing model
- convert the initial implementation of cgroup_taskset to use the new internal start/fill/end API
- convert the other two iterators (in separate patches) to use the new internal and external APIs.
Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |