Re: [PATCH 1/2] genirq: Extract irq_set_affinity_masks() from devm_platform_get_irqs_affinity()

John Garry
2022-02-17T17:07:59Z

On 16/02/2022 09:08, Marc Zyngier wrote:

Hi Marc,

> In order to better support drivers that deal with interrupts in a more
> "hands-on" way, extract the core of devm_platform_get_irqs_affinity()
> and expose it as irq_set_affinity_masks().
>
> This helper allows a driver to provide a set of wired interrupts that
> are to be configured as managed interrupts. As with the original helper,
> this is exported as EXPORT_SYMBOL_GPL.

I know you mentioned it in 2/2, but it would be interesting to see how
network controller drivers can handle the problem of missing in-flight
IO completions for managed irq shutdown. For storage controllers this is
all now safely handled in the block layer.

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Just a small comment, below.

Tested-by: John Garry <john.garry@huawei.com> #D05

Thanks,
John

> ---
> drivers/base/platform.c | 20 +++-----------------
> include/linux/interrupt.h | 8 ++++++++
> kernel/irq/affinity.c | 27 +++++++++++++++++++++++++++
> 3 files changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 6cb04ac48bf0..b363cf6ce5be 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -335,7 +335,6 @@ int devm_platform_get_irqs_affinity(struct platform_device *dev,
> int **irqs)
> {
> struct irq_affinity_devres *ptr;
> - struct irq_affinity_desc *desc;
> size_t size;
> int i, ret, nvec;
>
> @@ -376,31 +375,18 @@ int devm_platform_get_irqs_affinity(struct platform_device *dev,
> ptr->irq[i] = irq;
> }
>
> - desc = irq_create_affinity_masks(nvec, affd);
> - if (!desc) {
> - ret = -ENOMEM;
> + ret = irq_set_affinity_masks(affd, ptr->irq, nvec);
> + if (ret) {
> + dev_err(&dev->dev, "failed to update affinity descriptors (%d)\n", ret);
> goto err_free_devres;
> }
>
> - for (i = 0; i < nvec; i++) {
> - ret = irq_update_affinity_desc(ptr->irq[i], &desc[i]);
> - if (ret) {
> - dev_err(&dev->dev, "failed to update irq%d affinity descriptor (%d)\n",
> - ptr->irq[i], ret);
> - goto err_free_desc;
> - }
> - }
> -
> devres_add(&dev->dev, ptr);
>
> - kfree(desc);
> -
> *irqs = ptr->irq;
>
> return nvec;
>
> -err_free_desc:
> - kfree(desc);
> err_free_devres:
> devres_free(ptr);
> return ret;
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 9367f1cb2e3c..6bfce96206f8 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -381,6 +381,8 @@ irq_create_affinity_masks(unsigned int nvec, struct irq_affinity *affd);
> unsigned int irq_calc_affinity_vectors(unsigned int minvec, unsigned int maxvec,
> const struct irq_affinity *affd);
>
> +int irq_set_affinity_masks(struct irq_affinity *affd, int *irqs, int nvec);
> +
> #else /* CONFIG_SMP */
>
> static inline int irq_set_affinity(unsigned int irq, const struct cpumask *m)
> @@ -443,6 +445,12 @@ irq_calc_affinity_vectors(unsigned int minvec, unsigned int maxvec,
> return maxvec;
> }
>
> +static inline int
> +irq_set_affinity_masks(struct irq_affinity *affd, int *irqs, int nvec)
> +{
> + return -EINVAL;
> +}
> +
> #endif /* CONFIG_SMP */
>
> /*
> diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
> index f7ff8919dc9b..c0f868cd5b87 100644
> --- a/kernel/irq/affinity.c
> +++ b/kernel/irq/affinity.c
> @@ -512,3 +512,30 @@ unsigned int irq_calc_affinity_vectors(unsigned int minvec, unsigned int maxvec,
>
> return resv + min(set_vecs, maxvec - resv);
> }
> +
> +/*
> + * irq_set_affinity_masks - Set the affinity masks of a number of interrupts
> + * for multiqueue spreading
> + * @affd: Description of the affinity requirements
> + * @irqs: An array of interrupt numbers
> + * @nvec: The total number of interrupts
> + */
> +int irq_set_affinity_masks(struct irq_affinity *affd, int *irqs, int nvec)
> +{
> + struct irq_affinity_desc *desc;
> + int i, err = 0;

nit: it might be worth doing something similar to how
pci_alloc_irq_vectors_affinity() handles sets with no pre- and
post-vectors with msi_default_affd

> +
> + desc = irq_create_affinity_masks(nvec, affd);
> + if (!desc)
> + return -ENOMEM;
> +
> + for (i = 0; i < nvec; i++) {
> + err = irq_update_affinity_desc(irqs[i], desc + i);
> + if (err)
> + break;
> + }
> +
> + kfree(desc);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(irq_set_affinity_masks);