On Fri, 06 Aug 2010 10:38:24 -0600 Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: > On Fri, 2010-08-06 at 09:34 -0700, Sridhar Samudrala wrote: > > On 8/5/2010 3:59 PM, Michael S. Tsirkin wrote: > > > cgroup_attach_task_current_cg API that have upstream is backwards: we > > > really need an API to attach to the cgroups from another process A to > > > the current one. > > > > > > In our case (vhost), a priveledged user wants to attach it's task to cgroups > > > from a less priveledged one, the API makes us run it in the other > > > task's context, and this fails. > > > > > > So let's make the API generic and just pass in 'from' and 'to' tasks. > > > Add an inline wrapper for cgroup_attach_task_current_cg to avoid > > > breaking bisect. > > > > > > Signed-off-by: Michael S. Tsirkin<mst@xxxxxxxxxx> > > > --- > > > > > > Paul, Li, Sridhar, could you please review the following > > > patch? > > > > > > I only compile-tested it due to travel, but looks > > > straight-forward to me. > > > Alex Williamson volunteered to test and report the results. > > > Sending out now for review as I might be offline for a bit. > > > Will only try to merge when done, obviously. > > > > > > If OK, I would like to merge this through -net tree, > > > together with the patch fixing vhost-net. > > > Let me know if that sounds ok. > > > > > > Thanks! > > > > > > This patch is on top of net-next, it is needed for fix > > > vhost-net regression in net-next, where a non-priveledged > > > process can't enable the device anymore: > > > > > > when qemu uses vhost, inside the ioctl call it > > > creates a thread, and tries to add > > > this thread to the groups of current, and it fails. > > > But we control the thread, so to solve the problem, > > > we really should tell it 'connect to out cgroups'. So am I correct to assume that this change is now needed in 2.6.36, and unneeded in 2.6.35? Can it affect the userspace<->kernel API in amy manner? If so, it should be backported into earlier kernels to reduce the number of incompatible kernels out there. Paul, did you have any comments? I didn't see any update in response to the minor review comments, so... include/linux/cgroup.h | 1 + kernel/cgroup.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff -puN include/linux/cgroup.h~cgroups-fix-api-thinko-fix include/linux/cgroup.h --- a/include/linux/cgroup.h~cgroups-fix-api-thinko-fix +++ a/include/linux/cgroup.h @@ -579,6 +579,7 @@ void cgroup_iter_end(struct cgroup *cgrp int cgroup_scan_tasks(struct cgroup_scanner *scan); int cgroup_attach_task(struct cgroup *, struct task_struct *); int cgroup_attach_task_all(struct task_struct *from, struct task_struct *); + static inline int cgroup_attach_task_current_cg(struct task_struct *tsk) { return cgroup_attach_task_all(current, tsk); diff -puN kernel/cgroup.c~cgroups-fix-api-thinko-fix kernel/cgroup.c --- a/kernel/cgroup.c~cgroups-fix-api-thinko-fix +++ a/kernel/cgroup.c @@ -1798,13 +1798,13 @@ out: int cgroup_attach_task_all(struct task_struct *from, struct task_struct *tsk) { struct cgroupfs_root *root; - struct cgroup *cur_cg; int retval = 0; cgroup_lock(); for_each_active_root(root) { - cur_cg = task_cgroup_from_root(from, root); - retval = cgroup_attach_task(cur_cg, tsk); + struct cgroup *from_cg = task_cgroup_from_root(from, root); + + retval = cgroup_attach_task(from_cg, tsk); if (retval) break; } _ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html