Re: [Xen-devel] [PATCH 4/9] x86/IRQ: desc->affinity should strictly represent the requested value

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.05.19 at 18:21, <roger.pau@xxxxxxxxxx> wrote:
> On Mon, Apr 29, 2019 at 05:24:39AM -0600, Jan Beulich wrote:
>> desc->arch.cpu_mask reflects the actual set of target CPUs. Don't ever
>> fiddle with desc->affinity itself, except to store caller requested
>> values.
>> 
>> This renders both set_native_irq_info() uses (which weren't using proper
>> locking anyway) redundant - drop the function altogether.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> --- a/xen/arch/x86/irq.c
>> +++ b/xen/arch/x86/irq.c
>> @@ -572,11 +572,16 @@ int assign_irq_vector(int irq, const cpu
>>  
>>      spin_lock_irqsave(&vector_lock, flags);
>>      ret = __assign_irq_vector(irq, desc, mask ?: TARGET_CPUS);
>> -    if (!ret) {
>> +    if ( !ret )
>> +    {
>>          ret = desc->arch.vector;
>> -        cpumask_copy(desc->affinity, desc->arch.cpu_mask);
>> +        if ( mask )
>> +            cpumask_copy(desc->affinity, mask);
>> +        else
>> +            cpumask_setall(desc->affinity);
> 
> I guess it's fine to use setall instead of copying the cpu online map
> here?

It's not only fine, it's actually one of the goals: This way you can set
affinities such that they won't need adjustment after bringing up
another CPU. I've added a respective sentence to the description.

> AFAICT __assign_irq_vector already filters offline CPUs from the
> passed mask.

Indeed. And all other involved code should, too, by now. I think
there is at least one place left somewhere where the online map is
used for setting affinities, but I suppose this can be dealt with at
another time.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel