[PATCH net 0/4] eth: bnxt: fix IRQ notifier bugs

[PATCH net 0/4] eth: bnxt: fix IRQ notifier bugs

[Jakub Kicinski]

I was trying to make bnxt preserve IRQ mappings across reconfiguration.
While hacking on that I noticed 2 bugs in the notifiers that should
probably be fixed before development work.

First one is simple - TPH recofig makes aARFs not work. There can only
be one notifier per IRQ and TPH "steals" the callback from the rmap
updates. Fix by patches 1 and 2.

Second one is a deadlock between the affinity notifier and reconfig.
This one is a bit more involved (patch 3 and 4).

Unfortunately, I can't really verify the problem or test the fix.
I managed to get my hands on a system with an AMD Venice CPU which
is supposed to support TPH, but the ACPI is missing some bits to
actually advertise it. pcie_tph_get_cpu_st() returns -EINVAL.

This series would conflict with the affinity preservation which will
need to go to net-next, so I don't want to wait until I can test.

Jakub Kicinski (4):
  eth: bnxt: cancel IRQ notifier before freeing affinity mask
  eth: bnxt: keep the aRFS rmap updated when TPH is enabled
  eth: bnxt: decrease indent in bnxt_request_irq()
  eth: bnxt: avoid deadlock when canceling IRQ affinity notifier

 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   3 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 129 +++++++++++++++-------
 2 files changed, 91 insertions(+), 41 deletions(-)

-- 
2.55.0

[PATCH net 1/4] eth: bnxt: cancel IRQ notifier before freeing affinity mask

[Jakub Kicinski]

bnxt_irq_affinity_notify() copies into irq->cpu_mask.
Cancel the notifier before freeing irq->cpu_mask.

Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7513618793da..4a2a44ffe669 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11859,14 +11859,14 @@ static void bnxt_free_irq(struct bnxt *bp)
 
 		irq = &bp->irq_tbl[map_idx];
 		if (irq->requested) {
+			bnxt_release_irq_notifier(irq);
+
 			if (irq->have_cpumask) {
 				irq_update_affinity_hint(irq->vector, NULL);
 				free_cpumask_var(irq->cpu_mask);
 				irq->have_cpumask = 0;
 			}
 
-			bnxt_release_irq_notifier(irq);
-
 			free_irq(irq->vector, bp->bnapi[i]);
 		}
 
-- 
2.55.0

Re: [PATCH net 1/4] eth: bnxt: cancel IRQ notifier before freeing affinity mask

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

On Mon, Aug 3, 2026 at 12:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> bnxt_irq_affinity_notify() copies into irq->cpu_mask.
> Cancel the notifier before freeing irq->cpu_mask.
>
> Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 7513618793da..4a2a44ffe669 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -11859,14 +11859,14 @@ static void bnxt_free_irq(struct bnxt *bp)
>
>                 irq = &bp->irq_tbl[map_idx];
>                 if (irq->requested) {
> +                       bnxt_release_irq_notifier(irq);
> +

bnxt_release_irq_notifier() calls irq_set_affinity_notifier() to set
the notifier to NULL.  But the call is not synchronous, right?  A
pending notifier is still possible after irq_set_affinity_notifier()
returns, right?

>                         if (irq->have_cpumask) {
>                                 irq_update_affinity_hint(irq->vector, NULL);
>                                 free_cpumask_var(irq->cpu_mask);
>                                 irq->have_cpumask = 0;
>                         }
>
> -                       bnxt_release_irq_notifier(irq);
> -
>                         free_irq(irq->vector, bp->bnapi[i]);
>                 }

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net 1/4] eth: bnxt: cancel IRQ notifier before freeing affinity mask

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

On Mon, Aug 3, 2026 at 1:01 PM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Mon, Aug 3, 2026 at 12:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > bnxt_irq_affinity_notify() copies into irq->cpu_mask.
> > Cancel the notifier before freeing irq->cpu_mask.
> >
> > Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index 7513618793da..4a2a44ffe669 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > @@ -11859,14 +11859,14 @@ static void bnxt_free_irq(struct bnxt *bp)
> >
> >                 irq = &bp->irq_tbl[map_idx];
> >                 if (irq->requested) {
> > +                       bnxt_release_irq_notifier(irq);
> > +
>
> bnxt_release_irq_notifier() calls irq_set_affinity_notifier() to set
> the notifier to NULL.  But the call is not synchronous, right?  A
> pending notifier is still possible after irq_set_affinity_notifier()
> returns, right?

Nevermind.  It is synchronous so there is no issue.

Reviewed-by: Michael Chan <michael.chan@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

[PATCH net 2/4] eth: bnxt: keep the aRFS rmap updated when TPH is enabled

[Jakub Kicinski]

The TPH support must have broken aRFS in bnxt. IRQ can only have one
notifier, so installing the TPH notifier is overriding the one implicitly
installed by irq_cpu_rmap_add().

Make sure we call cpu_rmap_update() from the TPH notifier.

We need to be careful with the ordering and not free the rmap
until we unregistered the notifier. Note that moving the rmap
freeing after the early return in bnxt_free_irq() is fine -
there's no path that could leave rmap with irq_tbl being NULL.

Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4a2a44ffe669..47bea6d8ee3e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11774,6 +11774,16 @@ static void bnxt_irq_affinity_notify(struct irq_affinity_notify *notify,
 
 	irq = container_of(notify, struct bnxt_irq, affinity_notify);
 
+#ifdef CONFIG_RFS_ACCEL
+	if (irq->bp->dev->rx_cpu_rmap && irq->ring_nr < irq->bp->rx_nr_rings) {
+		err = cpu_rmap_update(irq->bp->dev->rx_cpu_rmap, irq->ring_nr,
+				      mask);
+		if (err)
+			netdev_warn(irq->bp->dev,
+				    "aRFS rmap update failed: %d\n", err);
+	}
+#endif
+
 	if (!irq->bp->tph_mode)
 		return;
 
@@ -11847,10 +11857,6 @@ static void bnxt_free_irq(struct bnxt *bp)
 	struct bnxt_irq *irq;
 	int i;
 
-#ifdef CONFIG_RFS_ACCEL
-	free_irq_cpu_rmap(bp->dev->rx_cpu_rmap);
-	bp->dev->rx_cpu_rmap = NULL;
-#endif
 	if (!bp->irq_tbl || !bp->bnapi)
 		return;
 
@@ -11876,6 +11882,11 @@ static void bnxt_free_irq(struct bnxt *bp)
 	/* Disable TPH support */
 	pcie_disable_tph(bp->pdev);
 	bp->tph_mode = 0;
+
+#ifdef CONFIG_RFS_ACCEL
+	free_irq_cpu_rmap(bp->dev->rx_cpu_rmap);
+	bp->dev->rx_cpu_rmap = NULL;
+#endif
 }
 
 static int bnxt_request_irq(struct bnxt *bp)
-- 
2.55.0

Re: [PATCH net 2/4] eth: bnxt: keep the aRFS rmap updated when TPH is enabled

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On Mon, Aug 3, 2026 at 12:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> The TPH support must have broken aRFS in bnxt. IRQ can only have one
> notifier, so installing the TPH notifier is overriding the one implicitly
> installed by irq_cpu_rmap_add().
>
> Make sure we call cpu_rmap_update() from the TPH notifier.
>
> We need to be careful with the ordering and not free the rmap
> until we unregistered the notifier. Note that moving the rmap
> freeing after the early return in bnxt_free_irq() is fine -
> there's no path that could leave rmap with irq_tbl being NULL.
>
> Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Michael Chan <michael.chan@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

[PATCH net 3/4] eth: bnxt: decrease indent in bnxt_request_irq()

[Jakub Kicinski]

bnxt_request_irq() has unnecessary level of indentation.
Use continue instead. No need to re-fetch NUMA node for
each IRQ, move to the function level.

No functional changes.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 50 +++++++++++------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 47bea6d8ee3e..202a16170141 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11891,6 +11891,7 @@ static void bnxt_free_irq(struct bnxt *bp)
 
 static int bnxt_request_irq(struct bnxt *bp)
 {
+	const int numa_node = dev_to_node(&bp->pdev->dev);
 	struct cpu_rmap *rmap = NULL;
 	int i, j, rc = 0;
 	unsigned long flags = 0;
@@ -11913,6 +11914,7 @@ static int bnxt_request_irq(struct bnxt *bp)
 	for (i = 0, j = 0; i < bp->cp_nr_rings; i++) {
 		int map_idx = bnxt_cp_num_to_irq_num(bp, i);
 		struct bnxt_irq *irq = &bp->irq_tbl[map_idx];
+		u16 tag;
 
 		if (IS_ENABLED(CONFIG_RFS_ACCEL) &&
 		    rmap && bp->bnapi[i]->rx_ring) {
@@ -11931,33 +11933,31 @@ static int bnxt_request_irq(struct bnxt *bp)
 		netif_napi_set_irq_locked(&bp->bnapi[i]->napi, irq->vector);
 		irq->requested = 1;
 
-		if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
-			int numa_node = dev_to_node(&bp->pdev->dev);
-			u16 tag;
+		if (!zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL))
+			continue;
 
-			irq->have_cpumask = 1;
-			irq->msix_nr = map_idx;
-			irq->ring_nr = i;
-			cpumask_set_cpu(cpumask_local_spread(i, numa_node),
-					irq->cpu_mask);
-			rc = irq_update_affinity_hint(irq->vector, irq->cpu_mask);
-			if (rc) {
-				netdev_warn(bp->dev,
-					    "Update affinity hint failed, IRQ = %d\n",
-					    irq->vector);
-				break;
-			}
-
-			bnxt_register_irq_notifier(bp, irq);
-
-			/* Init ST table entry */
-			if (pcie_tph_get_cpu_st(irq->bp->pdev, TPH_MEM_TYPE_VM,
-						cpumask_first(irq->cpu_mask),
-						&tag))
-				continue;
-
-			pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag);
+		irq->have_cpumask = 1;
+		irq->msix_nr = map_idx;
+		irq->ring_nr = i;
+		cpumask_set_cpu(cpumask_local_spread(i, numa_node),
+				irq->cpu_mask);
+		rc = irq_update_affinity_hint(irq->vector, irq->cpu_mask);
+		if (rc) {
+			netdev_warn(bp->dev,
+				    "Update affinity hint failed, IRQ = %d\n",
+				    irq->vector);
+			break;
 		}
+
+		bnxt_register_irq_notifier(bp, irq);
+
+		/* Init ST table entry */
+		if (pcie_tph_get_cpu_st(irq->bp->pdev, TPH_MEM_TYPE_VM,
+					cpumask_first(irq->cpu_mask),
+					&tag))
+			continue;
+
+		pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag);
 	}
 	return rc;
 }
-- 
2.55.0

Re: [PATCH net 3/4] eth: bnxt: decrease indent in bnxt_request_irq()

[Breno Leitao]

On Mon, Aug 03, 2026 at 12:31:34PM -0700, Jakub Kicinski wrote:
> bnxt_request_irq() has unnecessary level of indentation.
> Use continue instead. No need to re-fetch NUMA node for
> each IRQ, move to the function level.
> 
> No functional changes.

The transformation looks equivalent to me.

> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Breno Leitao <leitao@debian.org>

[PATCH net 4/4] eth: bnxt: avoid deadlock when canceling IRQ affinity notifier

[Jakub Kicinski]

Unregistering IRQ affinity notifiers waits for the callback synchronously.
bnxt takes the netdev instance lock in the notifier (to restart the queue)
and cancels the work under the same lock. This may obviously deadlock.

Move the restart to the async service task. The queue restart isn't
super time sensitive. Store the new TPH tag, schedule the task.
Safely canceling the service task is already ironed out.

In bnxt_request_irq() the order of registering notifier, affinity and
initial TPH programming has to be inverted. I think it was racy
previously since user may trigger an update as soon as notifier
is installed.

There's a small known gap - if pcie_tph_get_cpu_st() fails at init
and the target tag is 0 we may miss programming the entry.
This does not seem worth fixing, the code has skip-on-failure
all over the place, anyway.

Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  3 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 86 ++++++++++++++++-------
 2 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 6335dfc14c98..18eadbc491e7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1268,6 +1268,8 @@ struct bnxt_irq {
 	struct bnxt	*bp;
 	int		msix_nr;
 	int		ring_nr;
+	u16		tag;
+	u16		new_tag;
 	struct irq_affinity_notify affinity_notify;
 };
 
@@ -2641,6 +2643,7 @@ struct bnxt {
 #define BNXT_RING_COAL_NOW_SP_EVENT	17
 #define BNXT_FW_RESET_NOTIFY_SP_EVENT	18
 #define BNXT_FW_EXCEPTION_SP_EVENT	19
+#define BNXT_TPH_UPDATE_SP_EVENT	20
 #define BNXT_LINK_CFG_CHANGE_SP_EVENT	21
 #define BNXT_THERMAL_THRESHOLD_SP_EVENT	22
 #define BNXT_FW_ECHO_REQUEST_SP_EVENT	23
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 202a16170141..72d808c06def 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11770,12 +11770,13 @@ static void bnxt_irq_affinity_notify(struct irq_affinity_notify *notify,
 {
 	struct bnxt_irq *irq;
 	u16 tag;
-	int err;
 
 	irq = container_of(notify, struct bnxt_irq, affinity_notify);
 
 #ifdef CONFIG_RFS_ACCEL
 	if (irq->bp->dev->rx_cpu_rmap && irq->ring_nr < irq->bp->rx_nr_rings) {
+		int err;
+
 		err = cpu_rmap_update(irq->bp->dev->rx_cpu_rmap, irq->ring_nr,
 				      mask);
 		if (err)
@@ -11793,20 +11794,11 @@ static void bnxt_irq_affinity_notify(struct irq_affinity_notify *notify,
 		return;
 
 	if (pcie_tph_get_cpu_st(irq->bp->pdev, TPH_MEM_TYPE_VM,
-				cpumask_first(irq->cpu_mask), &tag))
+				cpumask_first(mask), &tag))
 		return;
 
-	if (pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag))
-		return;
-
-	netdev_lock(irq->bp->dev);
-	if (netif_running(irq->bp->dev)) {
-		err = netdev_rx_queue_restart(irq->bp->dev, irq->ring_nr);
-		if (err)
-			netdev_err(irq->bp->dev,
-				   "RX queue restart failed: err=%d\n", err);
-	}
-	netdev_unlock(irq->bp->dev);
+	WRITE_ONCE(irq->new_tag, tag);
+	bnxt_queue_sp_work(irq->bp, BNXT_TPH_UPDATE_SP_EVENT);
 }
 
 static void bnxt_irq_affinity_release(struct kref *ref)
@@ -11877,6 +11869,8 @@ static void bnxt_free_irq(struct bnxt *bp)
 		}
 
 		irq->requested = 0;
+		irq->tag = 0;
+		irq->new_tag = 0;
 	}
 
 	/* Disable TPH support */
@@ -11914,6 +11908,7 @@ static int bnxt_request_irq(struct bnxt *bp)
 	for (i = 0, j = 0; i < bp->cp_nr_rings; i++) {
 		int map_idx = bnxt_cp_num_to_irq_num(bp, i);
 		struct bnxt_irq *irq = &bp->irq_tbl[map_idx];
+		unsigned int cpu_num;
 		u16 tag;
 
 		if (IS_ENABLED(CONFIG_RFS_ACCEL) &&
@@ -11939,8 +11934,19 @@ static int bnxt_request_irq(struct bnxt *bp)
 		irq->have_cpumask = 1;
 		irq->msix_nr = map_idx;
 		irq->ring_nr = i;
-		cpumask_set_cpu(cpumask_local_spread(i, numa_node),
-				irq->cpu_mask);
+		cpu_num = cpumask_local_spread(i, numa_node);
+		cpumask_set_cpu(cpu_num, irq->cpu_mask);
+
+		/* Init ST table entry if we can get the mapping */
+		if (!pcie_tph_get_cpu_st(bp->pdev, TPH_MEM_TYPE_VM,
+					 cpu_num, &tag)) {
+			pcie_tph_set_st_entry(bp->pdev, irq->msix_nr, tag);
+			irq->tag = tag;
+			irq->new_tag = tag;
+		}
+
+		bnxt_register_irq_notifier(bp, irq);
+
 		rc = irq_update_affinity_hint(irq->vector, irq->cpu_mask);
 		if (rc) {
 			netdev_warn(bp->dev,
@@ -11948,16 +11954,6 @@ static int bnxt_request_irq(struct bnxt *bp)
 				    irq->vector);
 			break;
 		}
-
-		bnxt_register_irq_notifier(bp, irq);
-
-		/* Init ST table entry */
-		if (pcie_tph_get_cpu_st(irq->bp->pdev, TPH_MEM_TYPE_VM,
-					cpumask_first(irq->cpu_mask),
-					&tag))
-			continue;
-
-		pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag);
 	}
 	return rc;
 }
@@ -14460,6 +14456,43 @@ static void bnxt_rtnl_unlock_sp(struct bnxt *bp)
 	rtnl_unlock();
 }
 
+static void bnxt_tph_update(struct bnxt *bp)
+{
+	struct net_device *dev = bp->dev;
+	int i;
+
+	bnxt_lock_sp(bp);
+	if (!test_bit(BNXT_STATE_OPEN, &bp->state))
+		goto unlock;
+
+	for (i = 0; i < bp->rx_nr_rings; i++) {
+		struct bnxt_irq *irq;
+		int map_idx, err;
+		u16 tag;
+
+		map_idx = bnxt_cp_num_to_irq_num(bp, i);
+		irq = &bp->irq_tbl[map_idx];
+		tag = READ_ONCE(irq->new_tag);
+		if (irq->tag == tag)
+			continue;
+
+		if (pcie_tph_set_st_entry(bp->pdev, irq->msix_nr, tag))
+			continue;
+
+		err = netdev_rx_queue_restart(dev, irq->ring_nr);
+		if (err) {
+			netdev_err(dev, "RX queue restart failed: err=%d\n",
+				   err);
+			continue;
+		}
+
+		irq->tag = tag;
+	}
+
+unlock:
+	bnxt_unlock_sp(bp);
+}
+
 /* Only called from bnxt_sp_task() */
 static void bnxt_reset(struct bnxt *bp, bool silent)
 {
@@ -14884,6 +14917,9 @@ static void bnxt_sp_task(struct work_struct *work)
 			bnxt_devlink_health_fw_report(bp);
 	}
 
+	if (test_and_clear_bit(BNXT_TPH_UPDATE_SP_EVENT, &bp->sp_event))
+		bnxt_tph_update(bp);
+
 	smp_mb__before_atomic();
 	clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
 }
-- 
2.55.0

Re: [PATCH net 4/4] eth: bnxt: avoid deadlock when canceling IRQ affinity notifier

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On Mon, Aug 3, 2026 at 12:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Unregistering IRQ affinity notifiers waits for the callback synchronously.
> bnxt takes the netdev instance lock in the notifier (to restart the queue)
> and cancels the work under the same lock. This may obviously deadlock.
>
> Move the restart to the async service task. The queue restart isn't
> super time sensitive. Store the new TPH tag, schedule the task.
> Safely canceling the service task is already ironed out.
>
> In bnxt_request_irq() the order of registering notifier, affinity and
> initial TPH programming has to be inverted. I think it was racy
> previously since user may trigger an update as soon as notifier
> is installed.
>
> There's a small known gap - if pcie_tph_get_cpu_st() fails at init
> and the target tag is 0 we may miss programming the entry.
> This does not seem worth fixing, the code has skip-on-failure
> all over the place, anyway.
>
> Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Thanks for the patches.  We'll get the patches tested in our lab.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net 4/4] eth: bnxt: avoid deadlock when canceling IRQ affinity notifier

[Vishvambar Panth S]

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

Test summary: on an AMD EPYC 9335 CPU.
- Instrumented driver to see relevant debug logs - init/notify/update/teardown
- smp_affinity_list of irqs modified under active iperf3 traffic
(irqbalance also active)
- Affinity modify with concurrent 'ip link down' (~20 iterations): no
hang/deadlock
- aRFS/TPH coexistence: verified packets are correctly steered to the
relevant queues.

Tested-by: Vishvambar Panth S <vishvambar.panth-s@broadcom.com>

On Wed, Aug 5, 2026 at 11:32 AM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Mon, Aug 3, 2026 at 12:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > Unregistering IRQ affinity notifiers waits for the callback synchronously.
> > bnxt takes the netdev instance lock in the notifier (to restart the queue)
> > and cancels the work under the same lock. This may obviously deadlock.
> >
> > Move the restart to the async service task. The queue restart isn't
> > super time sensitive. Store the new TPH tag, schedule the task.
> > Safely canceling the service task is already ironed out.
> >
> > In bnxt_request_irq() the order of registering notifier, affinity and
> > initial TPH programming has to be inverted. I think it was racy
> > previously since user may trigger an update as soon as notifier
> > is installed.
> >
> > There's a small known gap - if pcie_tph_get_cpu_st() fails at init
> > and the target tag is 0 we may miss programming the entry.
> > This does not seem worth fixing, the code has skip-on-failure
> > all over the place, anyway.
> >
> > Fixes: c214410c47d6 ("bnxt_en: Add TPH support in BNXT driver")
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>
> Thanks for the patches.  We'll get the patches tested in our lab.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5493 bytes --]