Messages in this thread | | | Date | Fri, 18 Feb 2011 10:47:51 +0800 | From | Li Zefan <> | Subject | Re: [PATCH 3/4] cpuset: Fix unchecked calls to NODEMASK_ALLOC() |
| |
Paul Menage wrote: > On Thu, Feb 17, 2011 at 2:46 PM, Andrew Morton > <akpm@linux-foundation.org> wrote: >> On Thu, 17 Feb 2011 09:50:09 +0800 >> Li Zefan <lizf@cn.fujitsu.com> wrote: >> >>> +/* >>> + * In functions that can't propogate errno to users, to avoid declaring a >>> + * nodemask_t variable, and avoid using NODEMASK_ALLOC that can return >>> + * -ENOMEM, we use this global cpuset_mems. >>> + * >>> + * It should be used with cgroup_lock held. >> >> I'll do s/should/must/ - that would be a nasty bug. >> >> I'd be more comfortable about the maintainability of this optimisation >> if we had >> >> WARN_ON(!cgroup_is_locked()); >> >> at each site. >> > > Agreed - that was my first thought on reading the patch. How about: > > static nodemask_t *cpuset_static_nodemask() {
Then this should be 'noinline', otherwise we'll have one copy for each function that calls it.
> static nodemask_t nodemask; > WARN_ON(!cgroup_is_locked()); > return &nodemask; > } > > and then just call cpuset_static_nodemask() in the various locations > being patched? >
I think a defect of this is people might call it twice in one function but don't know it returns the same variable?
For example in cpuset_attach():
void cpuset_attach(...) { nodemask_t *from = cpuset_static_nodemask(); nodemask_t *to = cpuset_static_nodemask(); ... }
| |