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

Marc Zyngier
2022-02-17T17:17:18Z

Hi John,

On 2022-02-17 17:07, John Garry wrote:
> 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.

Do you have a pointer to this? It'd be interesting to see if there is
a common pattern.

>
>>
>> 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

Yes, good point. This would probably simplify most callers of this code.

Thanks,

M.
--
Jazz is not dead. It just smells funny...