On Mon, Jun 19, 2023 at 04:16:10PM -0700, Xin Li wrote:
> +/*
> + * Called with vector_lock held
> + */
Instead of comments like that, I tend to add a lockdep_assert*()
statement to the same effect. Which unlike comment actually validate the
claim and since it's code it tends to not go stale like comments do.
> +static void __vector_cleanup(struct vector_cleanup *cl, bool check_irr)
> {
> struct apic_chip_data *apicd;
> struct hlist_node *tmp;
> + bool rearm = false;
lockdep_assert_held(&vector_lock);
> + hlist_for_each_entry_safe(apicd, tmp, &cl->head, clist) {
> unsigned int irr, vector = apicd->prev_vector;
>
> /*
> * Paranoia: Check if the vector that needs to be cleaned
> + * up is registered at the APICs IRR. That's clearly a
> + * hardware issue if the vector arrived on the old target
> + * _after_ interrupts were disabled above. Keep @apicd
> + * on the list and schedule the timer again to give the CPU
> + * a chance to handle the pending interrupt.
> + *
> + * Do not check IRR when called from lapic_offline(), because
> + * fixup_irqs() was just called to scan IRR for set bits and
> + * forward them to new destination CPUs via IPIs.
> */
> + irr = check_irr ? apic_read(APIC_IRR + (vector / 32 * 0x10)) : 0;
> if (irr & (1U << (vector % 32))) {
> + pr_warn_once("Moved interrupt pending in old target APIC %u\n", apicd->irq);
> + rearm = true;
> continue;
> }
> free_moved_vector(apicd);
> }
>
> + /*
> + * Must happen under vector_lock to make the timer_pending() check
> + * in __vector_schedule_cleanup() race free against the rearm here.
> + */
> + if (rearm)
> + mod_timer(&cl->timer, jiffies + 1);
> +}
> +
> +static void vector_cleanup_callback(struct timer_list *tmr)
> +{
> + struct vector_cleanup *cl = container_of(tmr, typeof(*cl), timer);
> +
> + /* Prevent vectors vanishing under us */
> + raw_spin_lock_irq(&vector_lock);
> + __vector_cleanup(cl, true);
> + raw_spin_unlock_irq(&vector_lock);
> }
