Re: [PATCH v2 5/7] x86/irq: deal with old_cpu_mask for interrupts in movement in fixup_irqs()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jun 12, 2024 at 11:04:26AM +0200, Jan Beulich wrote:
> On 12.06.2024 10:47, Roger Pau Monné wrote:
> > On Tue, Jun 11, 2024 at 02:45:09PM +0200, Jan Beulich wrote:
> >> On 10.06.2024 16:20, Roger Pau Monne wrote:
> >>> Given the current logic it's possible for ->arch.old_cpu_mask to get out 
> >>> of
> >>> sync: if a CPU set in old_cpu_mask is offlined and then onlined
> >>> again without old_cpu_mask having been updated the data in the mask will 
> >>> no
> >>> longer be accurate, as when brought back online the CPU will no longer 
> >>> have
> >>> old_vector configured to handle the old interrupt source.
> >>>
> >>> If there's an interrupt movement in progress, and the to be offlined CPU 
> >>> (which
> >>> is the call context) is in the old_cpu_mask clear it and update the mask, 
> >>> so it
> >>> doesn't contain stale data.
> >>
> >> This imo is too __cpu_disable()-centric. In the code you cover the
> >> smp_send_stop() case afaict, where it's all _other_ CPUs which are being
> >> offlined. As we're not meaning to bring CPUs online again in that case,
> >> dealing with the situation likely isn't needed. Yet the description should
> >> imo at least make clear that the case was considered.
> > 
> > What about adding the following paragraph:
> 
> Sounds good, just maybe one small adjustment:
> 
> > Note that when the system is going down fixup_irqs() will be called by
> > smp_send_stop() from CPU 0 with a mask with only CPU 0 on it,
> > effectively asking to move all interrupts to the current caller (CPU
> > 0) which is the only CPU online.  In that case we don't care to
> 
> "... the only CPU to remain online."

Right, that's better.

Thanks, Roger.