[Devel] Re: [PATCH v5 3/3] cgroups: make procs file writable

[Ben Blum]

On Mon, Dec 27, 2010 at 02:53:47AM -0800, David Rientjes wrote:
> On Mon, 27 Dec 2010, Ben Blum wrote:
> 
> > > I'm not sure what the benefit of defining it as a macro would be.  You're 
> > > defining these statically allocated nodemasks so they have file scope, I 
> > > hope (so they can be shared amongst the users who synchronize on 
> > > cgroup_lock() already).
> > 
> > In the attach() case, yes, but in other cases I was thinking they could
> > be put on the stack if CONFIG_NODES_SHIFT < 8, and static but still
> > per-function otherwise. Or should all the functions share the same
> > global nodemask?
> > 
> 
> I think it would be appropriate to use a shared nodemask with file scope 
> whenever you have cgroup_lock() to avoid the unnecessary kmalloc() even 
> with GFP_KERNEL.  Cpusets are traditionally used on very large machines in 
> the first place, so there is a higher likelihood that 
> CONFIG_NODES_SHIFT > 8 whenever CONFIG_CPUSETS is enabled.
> 
> All users of NODEMASK_ALLOC() should be protected by cgroup_lock() other 
> than cpuset_sprintf_memlist(), right?  That should be the only remaining 
> user of NODEMASK_ALLOC() and works well since it can return -ENOMEM.

Just checked; that looks right. Perhaps I should add cgroup_is_locked()
in cgroup.c and BUG_ON() checks for it in those functions, too?

-- Ben
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers