diff mbox series

[net,1/2] net: mana: Fix hint value before free irq

Message ID 1674767085-18583-2-git-send-email-haiyangz@microsoft.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Fix usage of irq affinity_hint | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 24 this patch: 24
netdev/cc_maintainers fail 2 blamed authors not CCed: ssengar@linux.microsoft.com pabeni@redhat.com; 8 maintainers not CCed: sharmaajay@microsoft.com kuba@kernel.org ssengar@linux.microsoft.com leon@kernel.org pabeni@redhat.com wei.liu@kernel.org longli@microsoft.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 5 this patch: 5
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 22 this patch: 22
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Haiyang Zhang Jan. 26, 2023, 9:04 p.m. UTC
Need to clear affinity_hint before free_irq(), otherwise there is a
one-time warning and stack trace during module unloading.

To fix this bug, set affinity_hint to NULL before free as required.

Cc: stable@vger.kernel.org
Fixes: 71fa6887eeca ("net: mana: Assign interrupts to CPUs based on NUMA nodes")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/gdma_main.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Leon Romanovsky Jan. 29, 2023, 9:27 a.m. UTC | #1
On Thu, Jan 26, 2023 at 01:04:44PM -0800, Haiyang Zhang wrote:
> Need to clear affinity_hint before free_irq(), otherwise there is a
> one-time warning and stack trace during module unloading.

Please add this warning and stack trace to the commit message.

Thanks

> 
> To fix this bug, set affinity_hint to NULL before free as required.
> 
> Cc: stable@vger.kernel.org
> Fixes: 71fa6887eeca ("net: mana: Assign interrupts to CPUs based on NUMA nodes")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/net/ethernet/microsoft/mana/gdma_main.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index b144f2237748..3bae9d4c1f08 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1297,6 +1297,8 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
>  	for (j = i - 1; j >= 0; j--) {
>  		irq = pci_irq_vector(pdev, j);
>  		gic = &gc->irq_contexts[j];
> +
> +		irq_update_affinity_hint(irq, NULL);
>  		free_irq(irq, gic);
>  	}
>  
> @@ -1324,6 +1326,9 @@ static void mana_gd_remove_irqs(struct pci_dev *pdev)
>  			continue;
>  
>  		gic = &gc->irq_contexts[i];
> +
> +		/* Need to clear the hint before free_irq */
> +		irq_update_affinity_hint(irq, NULL);
>  		free_irq(irq, gic);
>  	}
>  
> -- 
> 2.25.1
>
Michael Kelley (LINUX) Jan. 29, 2023, 2:26 p.m. UTC | #2
From: LKML haiyangz <lkmlhyz@microsoft.com> On Behalf Of Haiyang Zhang Sent: Thursday, January 26, 2023 1:05 PM
> 
> Need to clear affinity_hint before free_irq(), otherwise there is a
> one-time warning and stack trace during module unloading.
> 
> To fix this bug, set affinity_hint to NULL before free as required.
> 
> Cc: stable@vger.kernel.org
> Fixes: 71fa6887eeca ("net: mana: Assign interrupts to CPUs based on NUMA nodes")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/net/ethernet/microsoft/mana/gdma_main.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index b144f2237748..3bae9d4c1f08 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1297,6 +1297,8 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
>  	for (j = i - 1; j >= 0; j--) {
>  		irq = pci_irq_vector(pdev, j);
>  		gic = &gc->irq_contexts[j];
> +
> +		irq_update_affinity_hint(irq, NULL);
>  		free_irq(irq, gic);
>  	}
> 
> @@ -1324,6 +1326,9 @@ static void mana_gd_remove_irqs(struct pci_dev *pdev)
>  			continue;
> 
>  		gic = &gc->irq_contexts[i];
> +
> +		/* Need to clear the hint before free_irq */
> +		irq_update_affinity_hint(irq, NULL);
>  		free_irq(irq, gic);
>  	}
> 
> --
> 2.25.1

I think this patch should be folded into the second patch of this series.  While
this patch makes the warning go away, it doesn't really solve any problems by
itself.  It just papers over the warning.  My first reaction on seeing this patch
is that the warning exists because the memory for the mask likely had
been incorrectly managed, which is exactly the case.  Since this patch doesn't
really fix a problem on its own, I'd say merge it into the second patch.

That's just my $.02.  If you really want to keep it separate, that's not a
blocker for me.

Michael
Haiyang Zhang Jan. 29, 2023, 6:51 p.m. UTC | #3
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Sunday, January 29, 2023 4:28 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
> <decui@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Paul Rosswurm
> <paulros@microsoft.com>; olaf@aepfle.de; vkuznets@redhat.com;
> davem@davemloft.net; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH net, 1/2] net: mana: Fix hint value before free irq
> 
> On Thu, Jan 26, 2023 at 01:04:44PM -0800, Haiyang Zhang wrote:
> > Need to clear affinity_hint before free_irq(), otherwise there is a
> > one-time warning and stack trace during module unloading.
> 
> Please add this warning and stack trace to the commit message.

Will do.

Thanks,
- Haiyang
Haiyang Zhang Jan. 29, 2023, 6:54 p.m. UTC | #4
> -----Original Message-----
> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> Sent: Sunday, January 29, 2023 9:26 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; Dexuan Cui
> <decui@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Paul Rosswurm
> <paulros@microsoft.com>; olaf@aepfle.de; vkuznets@redhat.com;
> davem@davemloft.net; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: RE: [PATCH net, 1/2] net: mana: Fix hint value before free irq
> 
> From: LKML haiyangz <lkmlhyz@microsoft.com> On Behalf Of Haiyang Zhang
> Sent: Thursday, January 26, 2023 1:05 PM
> >
> > Need to clear affinity_hint before free_irq(), otherwise there is a
> > one-time warning and stack trace during module unloading.
> >
> > To fix this bug, set affinity_hint to NULL before free as required.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 71fa6887eeca ("net: mana: Assign interrupts to CPUs based on NUMA
> nodes")
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> >  drivers/net/ethernet/microsoft/mana/gdma_main.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > index b144f2237748..3bae9d4c1f08 100644
> > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > @@ -1297,6 +1297,8 @@ static int mana_gd_setup_irqs(struct pci_dev
> *pdev)
> >  	for (j = i - 1; j >= 0; j--) {
> >  		irq = pci_irq_vector(pdev, j);
> >  		gic = &gc->irq_contexts[j];
> > +
> > +		irq_update_affinity_hint(irq, NULL);
> >  		free_irq(irq, gic);
> >  	}
> >
> > @@ -1324,6 +1326,9 @@ static void mana_gd_remove_irqs(struct pci_dev
> *pdev)
> >  			continue;
> >
> >  		gic = &gc->irq_contexts[i];
> > +
> > +		/* Need to clear the hint before free_irq */
> > +		irq_update_affinity_hint(irq, NULL);
> >  		free_irq(irq, gic);
> >  	}
> >
> > --
> > 2.25.1
> 
> I think this patch should be folded into the second patch of this series.  While
> this patch makes the warning go away, it doesn't really solve any problems by
> itself.  It just papers over the warning.  My first reaction on seeing this patch
> is that the warning exists because the memory for the mask likely had
> been incorrectly managed, which is exactly the case.  Since this patch doesn't
> really fix a problem on its own, I'd say merge it into the second patch.
> 
> That's just my $.02.  If you really want to keep it separate, that's not a
> blocker for me.

Will do.

Thanks,
- Haiyang
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index b144f2237748..3bae9d4c1f08 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1297,6 +1297,8 @@  static int mana_gd_setup_irqs(struct pci_dev *pdev)
 	for (j = i - 1; j >= 0; j--) {
 		irq = pci_irq_vector(pdev, j);
 		gic = &gc->irq_contexts[j];
+
+		irq_update_affinity_hint(irq, NULL);
 		free_irq(irq, gic);
 	}
 
@@ -1324,6 +1326,9 @@  static void mana_gd_remove_irqs(struct pci_dev *pdev)
 			continue;
 
 		gic = &gc->irq_contexts[i];
+
+		/* Need to clear the hint before free_irq */
+		irq_update_affinity_hint(irq, NULL);
 		free_irq(irq, gic);
 	}