Re: [PULL] cpumask: finally make them variable size w/ CPUMASK_OFFSTACK.

[KOSAKI Motohiro <kosaki.motohiro@gmail.com>]

(5/9/12 10:16 PM), Rusty Russell wrote:
> On Wed, 09 May 2012 21:32:57 -0400, KOSAKI Motohiro<kosaki.motohiro@gmail.com>  wrote:
>> (5/9/12 2:10 AM), Rusty Russell wrote:
>>> Hi Ingo,
>>>
>>>           I finally rebased this on top of your tip tree, and tested it
>>> locally.  Some more old-style cpumask usages have crept in, but it's a
>>> fairly simple series.
>>>
>>> The final result is that if you enable CONFIG_CPUMASK_OFFSTACK, then
>>> 'struct cpumask' becomes an undefined type.  You can't accidentally take
>>> the size of it, assign it, or pass it by value.  And thus it's safe for
>>> us to make it smaller if nr_cpu_ids<   NR_CPUS, as the final patch does.
>>>
>>> It unfortunately requires the lglock cleanup patch, which Al already has
>>> queued, so I've included it here.
>>
>> Hi
>>
>> Thanks this effort. This is very cleaner than I expected.
>> However I should NAK following one patch. sorry. because of, lru-drain is
>> called from memory reclaim context. It mean, additional allocation may not
>> work. Please just use bare NR_CPUS bitmap instead. space wasting is minor
>> issue than that.
>
> But if it fails the allocation, that's fine: we just send a few more
> IPIs to every CPU:
>
> +	if (!zalloc_cpumask_var(&cpus_with_pcps, GFP_KERNEL)) {
> +		on_each_cpu(drain_local_pages, NULL, 1);
> +		return;
> +	}
>
> We can do it the other way, but it sets a bad example, and after we get
> rid of cpumask, it becomes:
>
>          static DECLARE_BITMAP(cpus_with_pcps, NR_CPUS);
>
>          ......
>
>   		if (has_pcps)
> 			cpumask_set_cpu(cpu, to_cpumask(cpus_with_pcps));
> 		else
> 			cpumask_clear_cpu(cpu, to_cpumask(cpus_with_pcps));
>   	}
> 	on_each_cpu_mask(to_cpumask(cpus_with_pcps), drain_local_pages, NULL, 1);
>
> Or is there a reason we shouldn't even try to allocate here?

1) your code always use GFP_KERNEL. it is trouble maker when alloc_pages w/ GFP_ATOMIC.
2) When CONFIG_CPUMASK_OFFSTACK=n and NR_CPUS is relatively large, cpumask on stack may
cause stack overflow. because of, alloc_pages() can be called from very deep call stack.

Thought?