David Rientjes wrote:
> On Wed, 29 Dec 2010, Li Zefan wrote:
>
>>> 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.
>>>
>> Changing cpuset->mems_allowed is protected by both cgroup_mutex and
>> cpuset-specific lock (callback_mutex), so you can read it under either
>> lock, so NODEMASK_ALLOC() is not needed. See cpuset_sprintf_cpulist().
>>
>
> I'm not sure what you're saying. Cpusets needs to allocate nodemasks for
> certain functions and doing on the stack can be problemantic if
> CONFIG_NODES_SHIFT is large because of overflow. Thus, we can't have
> temporary nodemasks available on the stack where necessary in functions
> like cpuset_attach(), update_nodemask(), etc. that require them. The
> suggestion was to use a statically allocated "scratch" nodemask since
> these functions are all protected by cgroup_lock().
>
That's what we did for cpu masks :). See commit
2341d1b6598c7146d64a5050b53a72a5a819617f.
I made a patchset to remove on stack cpu masks.
What I meant is we don't have to allocate nodemasks in cpuset_sprintf_memlist().
This is sufficient:
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 4349935..a159612 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1620,20 +1620,12 @@ static int cpuset_sprintf_cpulist(char *page, struct cpu
static int cpuset_sprintf_memlist(char *page, struct cpuset *cs)
{
- NODEMASK_ALLOC(nodemask_t, mask, GFP_KERNEL);
int retval;
- if (mask == NULL)
- return -ENOMEM;
-
mutex_lock(&callback_mutex);
- *mask = cs->mems_allowed;
+ retval = nodelist_scnprintf(page, PAGE_SIZE, cs->mems_allowed);
mutex_unlock(&callback_mutex);
- retval = nodelist_scnprintf(page, PAGE_SIZE, *mask);
-
- NODEMASK_FREE(mask);
-
return retval;
}
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
