Re: [PATCH v2 5/7] s390/pci: Add error cleanup in zpci_directed_irq_init

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> If per-CPU airq_iv allocation fails in the loop, previously allocated
> vectors and arrays leak. Add proper error path to release all resources
> on failure.
> 
> Fixes: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
> Signed-off-by: Tobias Schumacher <ts@xxxxxxxxxxxxx>
> ---
>  arch/s390/pci/pci_irq.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index 81a27bf756a3..682bceb6525e 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -576,7 +576,7 @@ static int __init zpci_directed_irq_init(void)
>  
>  	zpci_dibv = kzalloc_objs(*zpci_dibv, num_possible_cpus());
>  	if (!zpci_dibv)
> -		return -ENOMEM;
> +		goto out_free_sbv;
>  
>  	for_each_possible_cpu(cpu) {
>  		/*
> @@ -589,13 +589,25 @@ static int __init zpci_directed_irq_init(void)
>  						AIRQ_IV_CACHELINE |
>  						(!cpu ? AIRQ_IV_ALLOC : 0), NULL);
>  		if (!zpci_dibv[cpu])
> -			return -ENOMEM;
> +			goto out_free_dibv;
>  	}
>  	on_each_cpu(cpu_enable_directed_irq, NULL, 1);
>  
>  	zpci_irq_chip.irq_set_affinity = zpci_set_irq_affinity;
>  
>  	return 0;
> +
> +out_free_dibv:
> +	for_each_possible_cpu(cpu) {
> +		if (zpci_dibv[cpu])
> +			airq_iv_release(zpci_dibv[cpu]);
> +	}
> +	kfree(zpci_dibv);
> +	zpci_dibv = NULL;
> +out_free_sbv:
> +	airq_iv_release(zpci_sbv);
> +	zpci_sbv = NULL;
> +	return -ENOMEM;

As Sashiko notes this doesn't undo the
zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib) and via the iib leaves
the zpci_sbv->vector that gets freed here set in hardware. While
Sashiko also notes this is an __init function and these allocations are
extremely unlikely to fail I think the rollback is still just nice
form. I think we could simply move the zpci_set_irq_ctrl() to after the
allocations but before the cpu_enable_directed_irq(). It also seems
more logical to set the interrupt mode only after memory has been set
up.

Thanks,
Niklas