Re: [PATCH 1/2] IRQ: allocate CPU masks dynamically

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 03/11/11 15:51, Jan Beulich wrote:
>>>> On 03.11.11 at 16:43, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 03/11/11 15:17, Jan Beulich wrote:
>>>>>> On 03.11.11 at 15:49, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>> On 03/11/11 14:26, Jan Beulich wrote:
>>>>> IRQ: allocate CPU masks dynamically
>>>>>
>>>>> This includes delaying the initialization of dynamically created IRQs
>>>>> until their actual first use and some further elimination of uses of
>>>>> struct irq_cfg.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>>> One query which may or may not affect the patch.  Would we get better
>>>> caching characteristics if all cpumasks were allocated in consecutive
>>>> memory, rather than having 3 individual allocs in arch_init_one_irq_desc ?
>>> That was what the first version of the patch did, rejected by Keir
>>> (and not liked too much by me either).
>>>
>>> Jan
>> My understanding of the objection was hiding the variables themselves as
>> an array in the code.
>>
>> An alternative approach such as alloc'ing 3*sizeof(cpu mask) (cache
>> aligned) and assigning the relevant pointers to the current
>> cpumask_var_t's would be a suitable approach which causes the cpumasks
>> to be in contiguous memory, but not changing how they are referenced in
>> the code.
> That would mean just open-coding what the former patch did by
> abstraction. In my opinion that is even worse - either we want a
> generally usable mechanism to do this, or we don't do it at all.
>
> Jan

That is an interesting point, but I dont think it is worse.

There are very few cases where we want to be doing this, so open coding
is not too bad.  With a comment explaining why, I would have thought it
would be fine.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com