lkml.org 
[lkml]   [2021]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] genirq/affinity: add helper of irq_affinity_calc_sets
On Thu, Jul 15, 2021 at 09:27:14AM -0500, Bjorn Helgaas wrote:
> On Thu, Jul 15, 2021 at 07:18:27PM +0800, Ming Lei wrote:
> > 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 <helgaas@kernel.org>
> > Cc: linux-pci@vger.kernel.org
> > Cc: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > drivers/pci/msi.c | 3 ++-
> > include/linux/interrupt.h | 7 +++++++
> > kernel/irq/affinity.c | 29 ++++++++++++++++++-----------
> > 3 files changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index 9232255c8515..3d6db20d1b2b 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);
> > + WARN_ON_ONCE(irq_affinity_calc_sets(1, affd));
>
> Hmmm. Not sure I like this yet:
>
> - I prefer required code to be on its own, not hidden inside a
> WARN() (personal preference, I know).
>
> - WARN() doesn't seem like the right thing here. I think this
> generates a backtrace but the driver that called this has no
> indication. Isn't the problem that a .calc_sets() method set
> "affd->nr_sets > IRQ_AFFINITY_MAX_SETS"?

Yes. When the warning is triggered, memory corruption may have been caused,
not sure if the indication is needed.

>
> It looks like those methods are supplied by drivers
> (nvme_calc_irq_sets(), csio_calc_sets()) and it seems like they
> should find out about this somehow.

Yeah. The WARN() here is just to report the bug earlier.


Thanks,
Ming

\
 
 \ /
  Last update: 2021-07-16 04:11    [W:0.086 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site