[V2] genirq/affinity: add helper of irq_affinity_calc_sets

Commit Message

Ming Lei July 20, 2021, 4:42 a.m. UTC

When driver requests to allocate irq affinity managed vectors,
pci_alloc_irq_vectors_affinity() may fallback to single vector
allocation. In this situation, we don't need to call
irq_create_affinity_masks for calling into ->calc_sets() for
avoiding potential memory leak, so add the helper for this purpose.

Fixes: c66d4bd110a1 ("genirq/affinity: Add new callback for (re)calculating interrupt sets")
Reported-by: Bjorn Helgaas <[e-mail]>
Cc: [e-mail]
Cc: Christoph Hellwig <[e-mail]>
Signed-off-by: Ming Lei <[e-mail]>
---
V2:
	- move WARN_ON_ONCE() into irq_affinity_calc_sets
	- don't install default calc_sets() callback as suggested by
	  Christoph

 drivers/pci/msi.c         |  3 ++-
 include/linux/interrupt.h |  7 +++++++
 kernel/irq/affinity.c     | 28 +++++++++++++++++-----------
 3 files changed, 26 insertions(+), 12 deletions(-)

Comments

Christoph Hellwig July 20, 2021, 7:50 a.m. UTC | #1

> +int irq_affinity_calc_sets(unsigned int affvecs, struct irq_affinity *affd)
> +{
> +	/*
> +	 * Simple invocations do not provide a calc_sets() callback. Call
> +	 * the generic one.
> +	 */
> +	if (!affd->calc_sets)
> +		default_calc_sets(affd, affvecs);
> +	else
> +		affd->calc_sets(affd, affvecs);

Nit: avoid pointless negations:

	if (affd->calc_sets)
		affd->calc_sets(affd, affvecs);
	else
		default_calc_sets(affd, affvecs);

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Bjorn Helgaas July 21, 2021, 12:02 a.m. UTC | #2

On Tue, Jul 20, 2021 at 09:50:25AM +0200, Christoph Hellwig wrote:
> > +int irq_affinity_calc_sets(unsigned int affvecs, struct irq_affinity *affd)
> > +{
> > +	/*
> > +	 * Simple invocations do not provide a calc_sets() callback. Call
> > +	 * the generic one.
> > +	 */
> > +	if (!affd->calc_sets)
> > +		default_calc_sets(affd, affvecs);
> > +	else
> > +		affd->calc_sets(affd, affvecs);
> 
> Nit: avoid pointless negations:
> 
> 	if (affd->calc_sets)
> 		affd->calc_sets(affd, affvecs);
> 	else
> 		default_calc_sets(affd, affvecs);

+1

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <[e-mail]>

Thanks for doing this!  For the PCI part:

Acked-by: Bjorn Helgaas <[e-mail]>

Patch

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 9232255c8515..4e6fbdf0741c 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1224,7 +1224,8 @@  int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 			 * for the single interrupt case.
 			 */
 			if (affd)
-				irq_create_affinity_masks(1, affd);
+				irq_affinity_calc_sets(1, affd);
+
 			pci_intx(dev, 1);
 			return 1;
 		}
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 2ed65b01c961..c7ff84d60465 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -340,6 +340,7 @@  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_affinity_calc_sets(unsigned int affvecs, struct irq_affinity *affd);
 
 #else /* CONFIG_SMP */
 
@@ -391,6 +392,12 @@  irq_calc_affinity_vectors(unsigned int minvec, unsigned int maxvec,
 	return maxvec;
 }
 
+static inline int irq_affinity_calc_sets(unsigned int affvecs,
+					 struct irq_affinity *affd)
+{
+	return 0;
+}
+
 #endif /* CONFIG_SMP */
 
 /*
diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
index 4d89ad4fae3b..addd04d68d42 100644
--- a/kernel/irq/affinity.c
+++ b/kernel/irq/affinity.c
@@ -405,6 +405,22 @@  static void default_calc_sets(struct irq_affinity *affd, unsigned int affvecs)
 	affd->set_size[0] = affvecs;
 }
 
+int irq_affinity_calc_sets(unsigned int affvecs, struct irq_affinity *affd)
+{
+	/*
+	 * Simple invocations do not provide a calc_sets() callback. Call
+	 * the generic one.
+	 */
+	if (!affd->calc_sets)
+		default_calc_sets(affd, affvecs);
+	else
+		affd->calc_sets(affd, affvecs);
+
+	if (WARN_ON_ONCE(affd->nr_sets > IRQ_AFFINITY_MAX_SETS))
+		return -ERANGE;
+	return 0;
+}
+
 /**
  * irq_create_affinity_masks - Create affinity masks for multiqueue spreading
  * @nvecs:	The total number of vectors
@@ -429,17 +445,7 @@  irq_create_affinity_masks(unsigned int nvecs, struct irq_affinity *affd)
 	else
 		affvecs = 0;
 
-	/*
-	 * Simple invocations do not provide a calc_sets() callback. Install
-	 * the generic one.
-	 */
-	if (!affd->calc_sets)
-		affd->calc_sets = default_calc_sets;
-
-	/* Recalculate the sets */
-	affd->calc_sets(affd, affvecs);
-
-	if (WARN_ON_ONCE(affd->nr_sets > IRQ_AFFINITY_MAX_SETS))
+	if (irq_affinity_calc_sets(affvecs, affd))
 		return NULL;
 
 	/* Nothing to assign? */