On 16.05.2024 17:56, Roger Pau Monné wrote:
> On Thu, May 16, 2024 at 05:00:54PM +0200, Jan Beulich wrote:
>> On 16.05.2024 15:22, Roger Pau Monne wrote:
>>> @@ -2576,7 +2576,12 @@ void fixup_irqs(const cpumask_t *mask, bool verbose)
>>> release_old_vec(desc);
>>> }
>>>
>>> - if ( !desc->action || cpumask_subset(desc->affinity, mask) )
>>> + /*
>>> + * Avoid shuffling the interrupt around if it's assigned to a CPU
>>> set
>>> + * that's all covered by the requested affinity mask.
>>> + */
>>> + cpumask_and(affinity, desc->arch.cpu_mask, &cpu_online_map);
>>> + if ( !desc->action || cpumask_subset(affinity, mask) )
>>> {
>>> spin_unlock(&desc->lock);
>>> continue;
>>[...]
>> In
>> which case cpumask_subset() is going to always return true with your
>> change in place, if I'm not mistaken. That seems to make your change
>> questionable. Yet with that I guess I'm overlooking something.)
>
> I might we wrong, but I think you are missing that the to be offlined
> CPU has been removed from cpu_online_map by the time it gets passed
> to fixup_irqs().
Just on this part (I'll need to take more time to reply to other parts):
No, I've specifically paid attention to that fact. Yet for this particular
observation of mine is doesn't matter. If mask == &cpu_online_map, then
no matter what is in cpu_online_map
cpumask_and(affinity, desc->arch.cpu_mask, &cpu_online_map);
will mask things down to a subset of cpu_online_map, and hence
if ( !desc->action || cpumask_subset(affinity, mask) )
(effectively being
if ( !desc->action || cpumask_subset(affinity, &cpu_online_map) )
) is nothing else than
if ( !desc->action || true )
. Yet that doesn't feel quite right.
Jan
