Re: [PATCH for-4.21 v3 1/2] x86/apic: Avoid infinite loop in io_apic_level_ack_pending()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.10.2025 23:04, Jason Andryuk wrote:
> io_apic_level_ack_pending() will end up in an infinite loop if
> entry->pin == -1.  entry does not change, so it will keep reading -1.
> 
> Convert to a proper for loop so that continue works.  Add a new helper,
> next_entry(), to handle advancing to the next irq_pin_list entry.
> 
> Noticed during code inspection.  The infinite loop was not observed.
> 
> Fixes: f821102450a1 ("x86: IRQ Migration logic enhancement.")
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> v3:
> const on next_entry() parameter
> Remove spaces inside for loop braces
> Remove inner if (!entry) check
> Expand commit message to state noticed during code inspection
> 
> v2:
> continue (not break) for pin == -1.
> 
> I added the next_entry() helper since putting the expression in the for
> loop is a little cluttered.  The helper can also be re-used for other
> instances within the file.
> ---
>  xen/arch/x86/io_apic.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index c384f10c1b..c35d611ecf 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -1586,20 +1586,24 @@ static int __init cf_check setup_ioapic_ack(const 
> char *s)
>  }
>  custom_param("ioapic_ack", setup_ioapic_ack);
>  
> +static struct irq_pin_list *next_entry(const struct irq_pin_list *entry)
> +{
> +    if ( !entry->next )
> +        return NULL;
> +
> +    return irq_2_pin + entry->next;
> +}

When replying to the v2 thread I hadn't spotted yet that a v3 was already
posted. As indicated, imo this name to too generic (now). I'd be happy to
make adjustments while committing, as long as we can agree on some less
generic name.

Jan