netdev - Re: [PATCH 1/2] genirq: Extract irq_set_affinity_masks() from devm_platform_get_irqs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Feb 2022 17:07:46 +0000
From:   John Garry <john.garry@...wei.com>
To:     Marc Zyngier <maz@...nel.org>, <linux-kernel@...r.kernel.org>,
        <netdev@...r.kernel.org>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Marcin Wojtas <mw@...ihalf.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>, <kernel-team@...roid.com>
Subject: Re: [PATCH 1/2] genirq: Extract irq_set_affinity_masks() from
 devm_platform_get_irqs_affinity()

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@...nel.org>

Just a small comment, below.

Tested-by: John Garry <john.garry@...wei.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);