OpenVZ Forum - RDF feed
type: channel
items: [RFC] Transactional CGroup task attachment
items: Re: [RFC] Transactional CGroup task attachment

[RFC] Transactional CGroup task attachment
type: item
description: This is an initial design for a transactional task attachment framework for cgroups. There are probably some potential simplications that I've missed, particularly in the area of locking. Comments appreciated. The Problem =========== Currently cgroups task movements are done in three phases 1) all subsystems in the destination cgroup get the chance to veto the movement (via their can_attach()) callback 2) task->cgroups is updated (while holding task->alloc_lock) 3) all subsystems get an attach() callback to let them perform any housekeeping updates The problems with this include: - There's no way to ensure that the result of can_attach() remains valid up until the attach() callback, unless any invalidating operations call cgroup_lock() to synchronize with the change. This is fine for something like cpusets, where invalidating operations are rare slow events like the user removing all cpus from a cpuset, or cpu hotplug triggering removal of a cpuset's last cpu. It's not so good for the virtual address space controller where the can_attach() check might be that the res_counter has enough space, and an invalidating operation might be another task in the cgroup allocating another page of virtual address space. - It doesn't handle the case of the proposed "cgroup.procs" file which can move multiple threads into a cgroup in one operation; the can_attach() and attach() calls should be able to atomically allow all or none of the threads to move. - it can create races around the time of the movement regarding to which cgroup a resource charge/uncharge should be assigned (e.g. between steps 2 and 3 new resource usage will be charged to the destination cgroup, but step 3 might involve migrating a charge equal to the task's resource usage from the old cgroup to the new, resulting in over/under-charges. Conceptual solution =================== In ideal terms, a solution for this problem would meet the following requirements: - support movement of an arbitrary set of threads betwe…
creator: Paul Menage
date: 2008-07-10T06:46:33-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: Paul M wrote: > So, thoughts? The cpuset portions seem reasonable to me, at first glance. But cpusets are easy, as you note. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.940.382.4214 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Jackson
date: 2008-07-10T11:23:06-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: Thank you for your effort. On Wed, 9 Jul 2008 23:46:33 -0700 "Paul Menage" <menage@google.com> wrote: > 3) memory > > Curently the memory cgroup only uses the mm->owner's cgroup at charge > time, and keeps a reference to the cgroup on the page. However, > patches have been proposed that would move all non-shared (page count > == 1) pages to the destination cgroup when the mm->owner moves to a > new cgroup. Since it's not possible to prevent page count changes > without locking all mms on the system, even this transaction approach > can't really give guarantees. However, something like the following > would probably be suitable. It's very similar to the memrlimit > approach, except for the fact that we have to handle the fact that the > number of pages we finally move might not be exactly the same as the > number of pages we thought we'd be moving. > > prepare_attach_sleep() { > down_read(&mm->mmap_sem); > if (mm->owner != state->task) return 0; > count = count_unshared_pages(mm); > // save the count charged to the new cgroup > state->subsys[memcgroup_subsys_id] = (void *)count; > if ((ret = res_counter_charge(state->dest, count)) { > up_read(&mm->mmap_sem); > } > return ret; > } > > commit_attach() { > if (mm->owner == state->task) { > final_count = move_unshared_pages(mm, state->dest); > res_counter_uncharge(state->src, final_count); > count = state->subsys[memcgroup_subsys_id]; > res_counter_force_charge(state->dest, final_count - count); > } > up_read(&mm->mmap_sem); > } > > abort_attach_sleep() { > if (mm->owner == state->task) { > count = state->subsys[memcgroup_subsys_id]; > res_counter_uncharge(state->dest, count); > } > up_read(&mm->mmap_sem); > } > At frist look, maybe works well. we need some special codes (to move resource) but that's all. My small concern is a state change between prepare_attach_sleep() -> commit_attach(). Hmm...but as you say, we cannot do down_write(mmap_sem). Maybe inserting some check codes to mem_cgroup_charge() to stop charge while…
creator: KAMEZAWA Hiroyuki
date: 2008-07-11T00:17:45-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: Paul Menage wrote: > This is an initial design for a transactional task attachment > framework for cgroups. There are probably some potential simplications > that I've missed, particularly in the area of locking. Comments > appreciated. > Though not so elegant, it sounds workable in practise. Support for CGROUP_ATTACH_PROCESS adds complexity and some subsystems may have to aware of this and write code to support it, but fortunately it seems only numtasks needs to deal with this. Or we can implement this new task attachment framework for CGROUP_ATTACH_THREAD mode only, to see how good it works, and later we add support for CGROUP_ATTACH_PROCESS. And I'll do testing if this gets implemented. :) _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Li Zefan
date: 2008-07-11T02:14:33-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Thu, Jul 10, 2008 at 7:14 PM, Li Zefan <lizf@cn.fujitsu.com> wrote: > > Support for CGROUP_ATTACH_PROCESS adds complexity and some subsystems > may have to aware of this and write code to support it, but fortunately > it seems only numtasks needs to deal with this. Or we can implement > this new task attachment framework for CGROUP_ATTACH_THREAD mode only, > to see how good it works, and later we add support for > CGROUP_ATTACH_PROCESS. Yes, that's the right way to go - but getting the CGROUP_ATTACH_PROCESS mode working is a prerequisite for adding the "procs" file. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-07-11T03:31:26-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: Quoting Paul Menage (menage@google.com): > This is an initial design for a transactional task attachment > framework for cgroups. There are probably some potential simplications > that I've missed, particularly in the area of locking. Comments > appreciated. > > The Problem > =========== > > Currently cgroups task movements are done in three phases > > 1) all subsystems in the destination cgroup get the chance to veto the > movement (via their can_attach()) callback > 2) task->cgroups is updated (while holding task->alloc_lock) > 3) all subsystems get an attach() callback to let them perform any > housekeeping updates > > The problems with this include: > > - There's no way to ensure that the result of can_attach() remains > valid up until the attach() callback, unless any invalidating > operations call cgroup_lock() to synchronize with the change. This is > fine for something like cpusets, where invalidating operations are > rare slow events like the user removing all cpus from a cpuset, or cpu > hotplug triggering removal of a cpuset's last cpu. It's not so good > for the virtual address space controller where the can_attach() check > might be that the res_counter has enough space, and an invalidating > operation might be another task in the cgroup allocating another page > of virtual address space. > > - It doesn't handle the case of the proposed "cgroup.procs" file which > can move multiple threads into a cgroup in one operation; the > can_attach() and attach() calls should be able to atomically allow all > or none of the threads to move. > > - it can create races around the time of the movement regarding to > which cgroup a resource charge/uncharge should be assigned (e.g. > between steps 2 and 3 new resource usage will be charged to the > destination cgroup, but step 3 might involve migrating a charge equal > to the task's resource usage from the old cgroup to the new, resulting > in over/under-charges. > > > Conceptual solution > =================== > > In id…
creator: serue
date: 2008-07-11T14:27:39-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, Jul 11, 2008 at 7:27 AM, Serge E. Hallyn <serue@us.ibm.com> wrote: > > It does feel like it may be too much designed for one particular user > (i.e. is there a reason not to expect a future cgroup to need a check > under a spinlock before a check under a mutex - say an i_sem - in the > can_attach sequence?), It would be fine as long as the code didn't want to *keep* holding the spinlock after the first check, while taking the mutex - and since that style of code is invalid under the existing locking rules, I don't see that as a problem. There's nothing to stop a prepare_attach_sleep() method from taking a spinlock as long as it releases it before it returns. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-07-11T15:23:22-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: Quoting Paul Menage (menage@google.com): > On Fri, Jul 11, 2008 at 7:27 AM, Serge E. Hallyn <serue@us.ibm.com> wrote: > > > > It does feel like it may be too much designed for one particular user > > (i.e. is there a reason not to expect a future cgroup to need a check > > under a spinlock before a check under a mutex - say an i_sem - in the > > can_attach sequence?), > > It would be fine as long as the code didn't want to *keep* holding the > spinlock after the first check, while taking the mutex - and since > that style of code is invalid under the existing locking rules, I > don't see that as a problem. There's nothing to stop a > prepare_attach_sleep() method from taking a spinlock as long as it > releases it before it returns. > > Paul Good point. For some stupid reason i was thinking don't take a spinlock at all. Have you started an implementation? thanks, -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-07-11T15:34:21-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, Jul 11, 2008 at 8:34 AM, Serge E. Hallyn <serue@us.ibm.com> wrote: > > Have you started an implementation? > No, not yet. I wanted to get general agreement that the API made sense first. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-07-11T15:36:32-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: Paul Menage wrote: > This is an initial design for a transactional task attachment > framework for cgroups. There are probably some potential simplications > that I've missed, particularly in the area of locking. Comments > appreciated. > > The Problem > =========== > > Currently cgroups task movements are done in three phases > > 1) all subsystems in the destination cgroup get the chance to veto the > movement (via their can_attach()) callback > 2) task->cgroups is updated (while holding task->alloc_lock) > 3) all subsystems get an attach() callback to let them perform any > housekeeping updates > > The problems with this include: > > - There's no way to ensure that the result of can_attach() remains > valid up until the attach() callback, unless any invalidating > operations call cgroup_lock() to synchronize with the change. This is > fine for something like cpusets, where invalidating operations are > rare slow events like the user removing all cpus from a cpuset, or cpu > hotplug triggering removal of a cpuset's last cpu. It's not so good > for the virtual address space controller where the can_attach() check > might be that the res_counter has enough space, and an invalidating > operation might be another task in the cgroup allocating another page > of virtual address space. > Precisely! > - It doesn't handle the case of the proposed "cgroup.procs" file which > can move multiple threads into a cgroup in one operation; the > can_attach() and attach() calls should be able to atomically allow all > or none of the threads to move. > > - it can create races around the time of the movement regarding to > which cgroup a resource charge/uncharge should be assigned (e.g. > between steps 2 and 3 new resource usage will be charged to the > destination cgroup, but step 3 might involve migrating a charge equal > to the task's resource usage from the old cgroup to the new, resulting > in over/under-charges. > > > Conceptual solution > =================== > > In ideal terms, …
creator: Balbir Singh
date: 2008-07-11T16:12:39-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, Jul 11, 2008 at 9:12 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > This would ideally be recursive mutexes, Linus does not like recursive mutexes. But we already have them in function, if not in name, with things like cpu_hotplug.lock, which is an open-coded reader-writer mutex with recursion. >> >> int prepare_attach_sleep(struct cgroup_attach_state *state); >> > > Is _sleep really required to be specfied? The function name sounds as if the > callback processor will sleep. I wasn't quite sure about the name of that method either. We could call it prepare_attach_maysleep(); just prepare_attach() seems a little under-descriptive. >> Called with group_mutex (which prevents any other task movement >> between cgroups) held plus any mutexes/semaphores taken by earlier >> subsystems's callbacks. >> > > This sounds almost like the BKL for cgroups :) Yes, cgroup_mutex is indeed currently the BKL for cgroups. I'm working separately to reduce that with things like the per-subsystem hierarchy_mutex, which will also protect against task movements. > > 1. Prepare_attach() the subsystem does it's or fails Did you miss a word here? > 2. If someone failed, send out failed notifications to successfull callbacks > 3. On receiving a failed notification (due to a different cgroup failure), > clients undo their operation (done in prepare_attach()) > 4. If all was successful, move the task and call attached() after the task is > attached. That's pretty much what we have - except that I've got two prepare_attach() methods to handle the case that some subsystems might need mutexes and others might need spinlocks in order to handle synchronization between the move and their resource charge/uncharge mechanisms - because any mutexes (and memory allocations) need to be handled before spinlocks. > > I read through the rest of it. The sleep/nosleep might make sense (to help the > task acquire the type of lock it wants to acquire), but isn't sleep a generic > case for nosleep as…
creator: Paul Menage
date: 2008-07-11T20:41:34-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Wed, 2008-07-09 at 23:46 -0700, Paul Menage wrote: > This is an initial design for a transactional task attachment > framework for cgroups. There are probably some potential simplications > that I've missed, particularly in the area of locking. Comments > appreciated. > > The Problem > =========== > > Currently cgroups task movements are done in three phases > > 1) all subsystems in the destination cgroup get the chance to veto the > movement (via their can_attach()) callback > 2) task->cgroups is updated (while holding task->alloc_lock) > 3) all subsystems get an attach() callback to let them perform any > housekeeping updates > > The problems with this include: > > - There's no way to ensure that the result of can_attach() remains > valid up until the attach() callback, unless any invalidating > operations call cgroup_lock() to synchronize with the change. This is > fine for something like cpusets, where invalidating operations are > rare slow events like the user removing all cpus from a cpuset, or cpu > hotplug triggering removal of a cpuset's last cpu. It's not so good > for the virtual address space controller where the can_attach() check > might be that the res_counter has enough space, and an invalidating > operation might be another task in the cgroup allocating another page > of virtual address space. > > - It doesn't handle the case of the proposed "cgroup.procs" file which > can move multiple threads into a cgroup in one operation; the > can_attach() and attach() calls should be able to atomically allow all > or none of the threads to move. > > - it can create races around the time of the movement regarding to > which cgroup a resource charge/uncharge should be assigned (e.g. > between steps 2 and 3 new resource usage will be charged to the > destination cgroup, but step 3 might involve migrating a charge equal > to the task's resource usage from the old cgroup to the new, resulting > in over/under-charges. I agree with Balbir: this is a good descript…
creator: Matt Helsley
date: 2008-07-12T00:03:54-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, Jul 11, 2008 at 5:03 PM, Matt Helsley <matthltc@us.ibm.com> wrote: >> struct cgroup_attach_state { > > nit: How about naming it cgroup_attach_request or > cgroup_attach_request_state? I suggest this because it's not really > "state" that's kept beyond the prepare-then-(commit|abort) sequence. State doesn't have to be long-lived to be state. But I'm not too worried about the exact name for it, if people have other preferences. > > What about the task->alloc_lock? Might that need to be taken by multiple > subsystems? See my next comment. My thought was that cgroups would take that anyway prior to calling prepare_attach_nosleep(), since it's a requirement for changing task->cgroups anyway. > > Rather than describing what might be called later for each API entry > separately it might be simpler to prefix the whole API/protocol > description with something like: > ====== > A successful return from prepare_X will cause abort_X to be called if > any of the prepatory calls fail. (where X is either sleep or nosleep) > > A successful return from prepare_X will cause commit to be called if all > of the prepatory calls succeed. (where X is either sleep or nosleep) > > Otherwise no calls to abort_X or commit will be made. (where X is either > sleep or nosleep) I'll play with working that into the description. > I think that's correct based on your descriptions. Of course changing > this only makes sense if this proposal will go into Documentation/ in > some form.. Yes, we'd definitely need to document this in some detail. > > Allowing prepare_X to hold locks when it has exitted seems ripe for > introducing two separate subsystems that inadvertently take locks out of > order. Yes, but I'm not sure that there's much that we can do about that. If we want to guarantee to be able to rollback one subsystem when a later subsystem fails then we have to let the earlier subsystems continue to hold locks. Or is this too ambitious a goal to support? Paul ____________________________…
creator: Paul Menage
date: 2008-07-12T00:18:18-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, 2008-07-11 at 17:18 -0700, Paul Menage wrote: > On Fri, Jul 11, 2008 at 5:03 PM, Matt Helsley <matthltc@us.ibm.com> wrote: > >> struct cgroup_attach_state { > > > > nit: How about naming it cgroup_attach_request or > > cgroup_attach_request_state? I suggest this because it's not really > > "state" that's kept beyond the prepare-then-(commit|abort) sequence. > > State doesn't have to be long-lived to be state. But I'm not too > worried about the exact name for it, if people have other preferences. > > > > > What about the task->alloc_lock? Might that need to be taken by multiple > > subsystems? See my next comment. > > My thought was that cgroups would take that anyway prior to calling > prepare_attach_nosleep(), since it's a requirement for changing > task->cgroups anyway. Yeah, that makes sense. > > > > Rather than describing what might be called later for each API entry > > separately it might be simpler to prefix the whole API/protocol > > description with something like: > > ====== > > A successful return from prepare_X will cause abort_X to be called if > > any of the prepatory calls fail. (where X is either sleep or nosleep) > > > > A successful return from prepare_X will cause commit to be called if all > > of the prepatory calls succeed. (where X is either sleep or nosleep) > > > > Otherwise no calls to abort_X or commit will be made. (where X is either > > sleep or nosleep) > > I'll play with working that into the description. > > > I think that's correct based on your descriptions. Of course changing > > this only makes sense if this proposal will go into Documentation/ in > > some form.. > > Yes, we'd definitely need to document this in some detail. > > > > > Allowing prepare_X to hold locks when it has exitted seems ripe for > > introducing two separate subsystems that inadvertently take locks out of > > order. > > Yes, but I'm not sure that there's much that we can do about that. If > we want to guarantee to be able to rollback one subsystem whe…
creator: Matt Helsley
date: 2008-07-12T00:48:14-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, Jul 11, 2008 at 5:48 PM, Matt Helsley <matthltc@us.ibm.com> wrote: > > I can't see a better way to support that goal and it doesn't seem > overly ambitious to me. Just needs a somewhat specific test > configuration for new subsystem patches to detect the deadlock issue. Right - but all non-trivial development should be done with lockdep enabled anyway. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-07-12T01:49:17-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, 2008-07-11 at 18:49 -0700, Paul Menage wrote: > On Fri, Jul 11, 2008 at 5:48 PM, Matt Helsley <matthltc@us.ibm.com> wrote: > > > > I can't see a better way to support that goal and it doesn't seem > > overly ambitious to me. Just needs a somewhat specific test > > configuration for new subsystem patches to detect the deadlock issue. > > Right - but all non-trivial development should be done with lockdep > enabled anyway. > > Paul True, though I don't think it's as simple as just enabling lockdep. My understanding is you won't be able to determine if locks could ever be taken out of order unless all of the cgroup systems are enabled and they are all in the same cgroup hierarchy. Cheers, -Matt Helsley _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Matt Helsley
date: 2008-07-12T02:03:36-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, Jul 11, 2008 at 7:03 PM, Matt Helsley <matthltc@us.ibm.com> wrote: > > True, though I don't think it's as simple as just enabling lockdep. My > understanding is you won't be able to determine if locks could ever be > taken out of order unless all of the cgroup systems are enabled and they > are all in the same cgroup hierarchy. I wonder how hard it would be to extend lockdep to give a "signature" of locking operations from point A to point B? Then you could just compare a new subsystem with all existing subsystems without actually running with it. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-07-12T02:09:42-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, 11 Jul 2008 09:20:58 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > Thank you for your effort. > > On Wed, 9 Jul 2008 23:46:33 -0700 > "Paul Menage" <menage@google.com> wrote: > > 3) memory > > > > Curently the memory cgroup only uses the mm->owner's cgroup at charge > > time, and keeps a reference to the cgroup on the page. However, > > patches have been proposed that would move all non-shared (page count > > == 1) pages to the destination cgroup when the mm->owner moves to a > > new cgroup. Since it's not possible to prevent page count changes > > without locking all mms on the system, even this transaction approach > > can't really give guarantees. However, something like the following > > would probably be suitable. It's very similar to the memrlimit > > approach, except for the fact that we have to handle the fact that the > > number of pages we finally move might not be exactly the same as the > > number of pages we thought we'd be moving. > > > > prepare_attach_sleep() { > > down_read(&mm->mmap_sem); > > if (mm->owner != state->task) return 0; > > count = count_unshared_pages(mm); > > // save the count charged to the new cgroup > > state->subsys[memcgroup_subsys_id] = (void *)count; > > if ((ret = res_counter_charge(state->dest, count)) { > > up_read(&mm->mmap_sem); > > } > > return ret; > > } > > > > commit_attach() { > > if (mm->owner == state->task) { > > final_count = move_unshared_pages(mm, state->dest); > > res_counter_uncharge(state->src, final_count); > > count = state->subsys[memcgroup_subsys_id]; > > res_counter_force_charge(state->dest, final_count - count); > > } > > up_read(&mm->mmap_sem); > > } > > > > abort_attach_sleep() { > > if (mm->owner == state->task) { > > count = state->subsys[memcgroup_subsys_id]; > > res_counter_uncharge(state->dest, count); > > } > > up_read(&mm->mmap_sem); > > } > > > > At frist look, maybe works well. we need some special codes (to move resource) > but that's all. > > My small concern is…
creator: Daisuke Nishimura
date: 2008-07-14T06:28:22-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Mon, 14 Jul 2008 15:28:22 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > On Fri, 11 Jul 2008 09:20:58 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > > Thank you for your effort. > > > > On Wed, 9 Jul 2008 23:46:33 -0700 > > "Paul Menage" <menage@google.com> wrote: > > > 3) memory > > > > > > Curently the memory cgroup only uses the mm->owner's cgroup at charge > > > time, and keeps a reference to the cgroup on the page. However, > > > patches have been proposed that would move all non-shared (page count > > > == 1) pages to the destination cgroup when the mm->owner moves to a > > > new cgroup. Since it's not possible to prevent page count changes > > > without locking all mms on the system, even this transaction approach > > > can't really give guarantees. However, something like the following > > > would probably be suitable. It's very similar to the memrlimit > > > approach, except for the fact that we have to handle the fact that the > > > number of pages we finally move might not be exactly the same as the > > > number of pages we thought we'd be moving. > > > > > > prepare_attach_sleep() { > > > down_read(&mm->mmap_sem); > > > if (mm->owner != state->task) return 0; > > > count = count_unshared_pages(mm); > > > // save the count charged to the new cgroup > > > state->subsys[memcgroup_subsys_id] = (void *)count; > > > if ((ret = res_counter_charge(state->dest, count)) { > > > up_read(&mm->mmap_sem); > > > } > > > return ret; > > > } > > > > > > commit_attach() { > > > if (mm->owner == state->task) { > > > final_count = move_unshared_pages(mm, state->dest); > > > res_counter_uncharge(state->src, final_count); > > > count = state->subsys[memcgroup_subsys_id]; > > > res_counter_force_charge(state->dest, final_count - count); > > > } > > > up_read(&mm->mmap_sem); > > > } > > > > > > abort_attach_sleep() { > > > if (mm->owner == state->task) { > > > count = state->subsys[memcgroup_subsys_id]; > > > res_counter_uncharge(state->de…
creator: KAMEZAWA Hiroyuki
date: 2008-07-14T07:50:51-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, 2008-07-11 at 17:03 -0700, Matt Helsley wrote: > On Wed, 2008-07-09 at 23:46 -0700, Paul Menage wrote: > > struct cgroup_attach_state { > > nit: How about naming it cgroup_attach_request or > cgroup_attach_request_state? I suggest this because it's not really > "state" that's kept beyond the prepare-then-(commit|abort) sequence. Other alternatives: cgroup_attach_context, cgroup_attach_txn _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Peter Zijlstra
date: 2008-07-14T10:50:01-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Fri, 2008-07-11 at 19:09 -0700, Paul Menage wrote: > On Fri, Jul 11, 2008 at 7:03 PM, Matt Helsley <matthltc@us.ibm.com> wrote: > > > > True, though I don't think it's as simple as just enabling lockdep. My > > understanding is you won't be able to determine if locks could ever be > > taken out of order unless all of the cgroup systems are enabled and they > > are all in the same cgroup hierarchy. > > I wonder how hard it would be to extend lockdep to give a "signature" > of locking operations from point A to point B? Then you could just > compare a new subsystem with all existing subsystems without actually > running with it. something like: http://lkml.org/lkml/2008/6/22/308 ? _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Peter Zijlstra
date: 2008-07-14T10:56:49-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Mon, 14 Jul 2008 16:54:44 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Mon, 14 Jul 2008 15:28:22 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > On Fri, 11 Jul 2008 09:20:58 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > > > Thank you for your effort. > > > > > > On Wed, 9 Jul 2008 23:46:33 -0700 > > > "Paul Menage" <menage@google.com> wrote: > > > > 3) memory > > > > > > > > Curently the memory cgroup only uses the mm->owner's cgroup at charge > > > > time, and keeps a reference to the cgroup on the page. However, > > > > patches have been proposed that would move all non-shared (page count > > > > == 1) pages to the destination cgroup when the mm->owner moves to a > > > > new cgroup. Since it's not possible to prevent page count changes > > > > without locking all mms on the system, even this transaction approach > > > > can't really give guarantees. However, something like the following > > > > would probably be suitable. It's very similar to the memrlimit > > > > approach, except for the fact that we have to handle the fact that the > > > > number of pages we finally move might not be exactly the same as the > > > > number of pages we thought we'd be moving. > > > > > > > > prepare_attach_sleep() { > > > > down_read(&mm->mmap_sem); > > > > if (mm->owner != state->task) return 0; > > > > count = count_unshared_pages(mm); > > > > // save the count charged to the new cgroup > > > > state->subsys[memcgroup_subsys_id] = (void *)count; > > > > if ((ret = res_counter_charge(state->dest, count)) { > > > > up_read(&mm->mmap_sem); > > > > } > > > > return ret; > > > > } > > > > > > > > commit_attach() { > > > > if (mm->owner == state->task) { > > > > final_count = move_unshared_pages(mm, state->dest); > > > > res_counter_uncharge(state->src, final_count); > > > > count = state->subsys[memcgroup_subsys_id]; > > > > res_counter_force_charge(state->dest, final_count - count); > > > > } > > > > up_read(&mm->…
creator: Daisuke Nishimura
date: 2008-07-14T12:36:09-00:00

Re: [RFC] Transactional CGroup task attachment
type: item
description: On Sun, Jul 13, 2008 at 11:28 PM, Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > - I think page reclaiming code decreases the memory charge > without holding mmap_sem(e.g. try_to_unmap(), __remove_mapping()). > Shouldn't we handle these cases? The prepare_attach_nosleep() call could take the res_counter's spinlock, which would lock out all other charges and uncharges until the transaction was completed; that might be enough for what you want. We'd need to export lock/unlock functions from res_counter. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-07-14T19:16:08-00:00