On 08/09/2017 09:06 AM, Michael Ellerman wrote:
> Cédric Le Goater <c...@kaod.org> writes:
>> When called from xive_irq_startup(), the size of the cpumask can be
>> larger than nr_cpu_ids. Most of time, its value is NR_CPUS (2048).
>
> Ugh, you're right.
>
> #define nr_cpumask_bits ((unsigned int)NR_CPUS)
> ...
> /**
> * cpumask_weight - Count of bits in *srcp
> * @srcp: the cpumask to count bits (< nr_cpu_ids) in.
> */
> static inline unsigned int cpumask_weight(const struct cpumask *srcp)
> {
> return bitmap_weight(cpumask_bits(srcp), nr_cpumask_bits);
> }
>
>
> I don't know what the comment on srcp is trying to say. It's not true
> that it only counts nr_cpu_ids worth of bits.
>
> So it does seem if we're passed a mask with > nr_cpu_ids bits set then
> cpumask_weight() will return > nr_cpu_ids, which is .. unhelpful.
>
>
> BUT, I don't see other code handling cpumask_weight() returning >
> nr_cpu_ids - at least I can't find any with some grepping.
>
>
> So what is going wrong here that we're being passed a mask with more
> than nr_cpu_ids bits set?
>
> I think the affinity mask is copied to the desc in desc_smp_init(), and
> the call chain will be:
>
> irq_create_mapping()
> -> irq_domain_alloc_descs()
> -> __irq_alloc_descs()
> -> alloc_descs()
> -> alloc_desc()
> -> desc_set_defaults()
> -> desc_smp_init()
>
> irq_create_mapping() is doing:
>
> virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node), NULL);
>
> Where the affinity mask is the NULL at the end.
>
> So presumably we're hitting the irq_default_affinity case here:
>
> static void desc_smp_init(struct irq_desc *desc, int node,
> const struct cpumask *affinity)
> {
> if (!affinity)
> affinity = irq_default_affinity;
> cpumask_copy(desc->irq_common_data.affinity, affinity);
>
>
> Which comes from:
>
> static void __init init_irq_default_affinity(void)
> {
> #ifdef CONFIG_CPUMASK_OFFSTACK
> if (!irq_default_affinity)
> zalloc_cpumask_var(&irq_default_affinity, GFP_NOWAIT);
> #endif
> if (cpumask_empty(irq_default_affinity))
> cpumask_setall(irq_default_affinity);
> }
>
> And cpumask_setall() will indeed set NR_CPUs bits.
>
>
> So that all seems sane, except that it does mean cpumask_weight() can
> return > nr_cpu_ids which is awkward.
>
> I guess this patch is a good fix, I'll expand the change log a bit.
Yes. Thanks for the digging. I didn't do as much.
Cheers,
C.
