[PATCH v2 00/14] NTB: ntb_transport: Miscellaneous fixes

[PATCH v2 00/14] NTB: ntb_transport: Miscellaneous fixes

[Koichiro Den]

Hi,

This series attempts to fix miscellaneous issues in ntb_transport. Some
of these were split from the direct TX/RX series v1 [1], as they stand
on their own. Based on v7.3-rc2.

Dave, could you please review this series and ack if it looks good to
you?

[1] https://lore.kernel.org/r/20260810165136.2292436-1-den@valinux.co.jp/
    (The v2 of [1] will be based off of this series to avoid conflicts.)

Best regards,
Koichiro
---
Changes in v2:
  - Address Sashiko feedback, including regressions introduced by v1,
    with new patches 5 and 10.
    Note that to keep the series manageable, this revision does not
    attempt to fix all the reported pre-existing issues.
  - Add a fix for client removal ordering (patch 14).
  - Reorder patches, moving v1 patch 4 after the fixes it depends on.
  - Add missing Cc: stable@vger.kernel.org.

v1: https://lore.kernel.org/r/20260907142429.951930-1-den@valinux.co.jp/


Koichiro Den (14):
  NTB: ntb_transport: Remove the device debugfs directory
  NTB: ntb_transport: Start TX offload thread after queue setup
  NTB: ntb_transport: Avoid deadlock when cancelling link work
  NTB: ntb_transport: Publish link state after QP setup
  NTB: ntb_transport: Avoid losing QP link-up requests
  NTB: ntb_transport: Clear link state before QP cleanup
  NTB: ntb_transport: Stop QP work before freeing a queue
  NTB: ntb_transport: Stop RX tasklet scheduling before freeing a queue
  NTB: ntb_transport: Drain RX tasklets during link cleanup
  NTB: ntb_transport: Wait for RX completions before resetting a QP
  NTB: ntb_transport: Prepare remote RX info accesses for MW teardown
  NTB: ntb_transport: Clear QP pointers when freeing an MW
  NTB: ntb_transport: Abort link setup on QP MW allocation failure
  NTB: ntb_transport: Remove clients before freeing transport resources

 drivers/ntb/ntb_transport.c | 232 +++++++++++++++++++++++++-----------
 1 file changed, 163 insertions(+), 69 deletions(-)

base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.51.0

[PATCH v2 01/14] NTB: ntb_transport: Remove the device debugfs directory

[Koichiro Den]

ntb_transport_free() removes QP debugfs directories but leaves the
device directory. On rebind, debugfs_create_dir() fails with -EEXIST
and QP statistics files are not recreated. Module unload masks this
by removing the entire debugfs tree.

To reproduce:

  # ls /sys/kernel/debug/ntb_transport/0001:10:00.0/
  qp0
  # echo 0001:10:00.0 > /sys/bus/ntb/drivers/ntb_transport/unbind
  # ls /sys/kernel/debug/ntb_transport/
  0001:10:00.0   <-- should not remain
  # echo 0001:10:00.0 > /sys/bus/ntb/drivers/ntb_transport/bind

  .. and then dmesg shows:
  debugfs: '0001:10:00.0' already exists in 'ntb_transport'

  # ls /sys/kernel/debug/ntb_transport/0001:10:00.0/
  (nothing)      <-- should be 'qp0'

Remove the device debugfs tree on teardown and probe failure.

Verified that unbind removes the directory and rebind recreates qp0.

Fixes: c8650fd03d32 ("NTB: Fix transport stats for multiple devices")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index f9caa1a653c5..3389d6ca9ebd 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1382,6 +1382,7 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
 err3:
 	ntb_clear_ctx(ndev);
 err2:
+	debugfs_remove_recursive(nt->debugfs_node_dir);
 	kfree(nt->qp_vec);
 err1:
 	while (i--) {
@@ -1401,6 +1402,8 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
 	u64 qp_bitmap_alloc;
 	int i;
 
+	debugfs_remove_recursive(nt->debugfs_node_dir);
+
 	ntb_transport_link_cleanup(nt);
 	cancel_work_sync(&nt->link_cleanup);
 	cancel_delayed_work_sync(&nt->link_work);
@@ -1412,7 +1415,6 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
 		qp = &nt->qp_vec[i];
 		if (qp_bitmap_alloc & BIT_ULL(i))
 			ntb_transport_free_queue(qp);
-		debugfs_remove_recursive(qp->debugfs_dir);
 	}
 
 	ntb_link_disable(ndev);
-- 
2.51.0

Re: [PATCH v2 01/14] NTB: ntb_transport: Remove the device debugfs directory

[Frank Li]

On Thu, Sep 10, 2026 at 01:08:23PM +0900, Koichiro Den wrote:
> ntb_transport_free() removes QP debugfs directories but leaves the
> device directory. On rebind, debugfs_create_dir() fails with -EEXIST
> and QP statistics files are not recreated. Module unload masks this
> by removing the entire debugfs tree.
>
> To reproduce:
>
>   # ls /sys/kernel/debug/ntb_transport/0001:10:00.0/
>   qp0
>   # echo 0001:10:00.0 > /sys/bus/ntb/drivers/ntb_transport/unbind
>   # ls /sys/kernel/debug/ntb_transport/
>   0001:10:00.0   <-- should not remain
>   # echo 0001:10:00.0 > /sys/bus/ntb/drivers/ntb_transport/bind
>
>   .. and then dmesg shows:
>   debugfs: '0001:10:00.0' already exists in 'ntb_transport'
>
>   # ls /sys/kernel/debug/ntb_transport/0001:10:00.0/
>   (nothing)      <-- should be 'qp0'
>
> Remove the device debugfs tree on teardown and probe failure.
>
> Verified that unbind removes the directory and rebind recreates qp0.
>
> Fixes: c8650fd03d32 ("NTB: Fix transport stats for multiple devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v2:
>   - No changes.
>
>  drivers/ntb/ntb_transport.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f9caa1a653c5..3389d6ca9ebd 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1382,6 +1382,7 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  err3:
>  	ntb_clear_ctx(ndev);
>  err2:
> +	debugfs_remove_recursive(nt->debugfs_node_dir);
>  	kfree(nt->qp_vec);
>  err1:
>  	while (i--) {
> @@ -1401,6 +1402,8 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
>  	u64 qp_bitmap_alloc;
>  	int i;
>
> +	debugfs_remove_recursive(nt->debugfs_node_dir);
> +
>  	ntb_transport_link_cleanup(nt);
>  	cancel_work_sync(&nt->link_cleanup);
>  	cancel_delayed_work_sync(&nt->link_work);
> @@ -1412,7 +1415,6 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
>  		qp = &nt->qp_vec[i];
>  		if (qp_bitmap_alloc & BIT_ULL(i))
>  			ntb_transport_free_queue(qp);
> -		debugfs_remove_recursive(qp->debugfs_dir);
>  	}
>
>  	ntb_link_disable(ndev);
> --
> 2.51.0
>

Re: [PATCH v2 01/14] NTB: ntb_transport: Remove the device debugfs directory

[Logan Gunthorpe]

On 2026-09-09 22:08, Koichiro Den wrote:
> ntb_transport_free() removes QP debugfs directories but leaves the
> device directory. On rebind, debugfs_create_dir() fails with -EEXIST
> and QP statistics files are not recreated. Module unload masks this
> by removing the entire debugfs tree.
> 
> To reproduce:
> 
>   # ls /sys/kernel/debug/ntb_transport/0001:10:00.0/
>   qp0
>   # echo 0001:10:00.0 > /sys/bus/ntb/drivers/ntb_transport/unbind
>   # ls /sys/kernel/debug/ntb_transport/
>   0001:10:00.0   <-- should not remain
>   # echo 0001:10:00.0 > /sys/bus/ntb/drivers/ntb_transport/bind
> 
>   .. and then dmesg shows:
>   debugfs: '0001:10:00.0' already exists in 'ntb_transport'
> 
>   # ls /sys/kernel/debug/ntb_transport/0001:10:00.0/
>   (nothing)      <-- should be 'qp0'
> 
> Remove the device debugfs tree on teardown and probe failure.
> 
> Verified that unbind removes the directory and rebind recreates qp0.
> 
> Fixes: c8650fd03d32 ("NTB: Fix transport stats for multiple devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Looks good to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 02/14] NTB: ntb_transport: Start TX offload thread after queue setup

[Koichiro Den]

ntb_transport_create_queue() starts the per-QP TX offload thread before
DMA mappings and queue entries are allocated. If later setup fails, the
error path returns the QP to the free bitmap without stopping the
thread. A retry can then reinitialize its waitqueue while the old thread
is still waiting on it.

Start the thread after queue setup.

Fixes: 322617a06c97 ("NTB: ntb_transport: Add 'tx_memcpy_offload' module option")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

NOTE: Originally submitted as part of the direct TX/RX series v1:
https://lore.kernel.org/r/20260810165136.2292436-4-den@valinux.co.jp/

 drivers/ntb/ntb_transport.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 3389d6ca9ebd..55a20ae9a85e 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -2055,20 +2055,6 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
 	qp->tx_handler = handlers->tx_handler;
 	qp->event_handler = handlers->event_handler;
 
-	init_waitqueue_head(&qp->tx_offload_wq);
-	if (tx_memcpy_offload) {
-		qp->tx_offload_thread = kthread_run(ntb_tx_memcpy_kthread, qp,
-						    "ntb-txcpy/%s/%u",
-						    pci_name(ndev->pdev), qp->qp_num);
-		if (IS_ERR(qp->tx_offload_thread)) {
-			dev_warn(&nt->ndev->dev,
-				 "tx memcpy offload thread creation failed: %ld; falling back to inline copy\n",
-				 PTR_ERR(qp->tx_offload_thread));
-			qp->tx_offload_thread = NULL;
-		}
-	} else
-		qp->tx_offload_thread = NULL;
-
 	dma_cap_zero(dma_mask);
 	dma_cap_set(DMA_MEMCPY, dma_mask);
 
@@ -2129,6 +2115,20 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
 			     &qp->tx_free_q);
 	}
 
+	init_waitqueue_head(&qp->tx_offload_wq);
+	qp->tx_offload_thread = NULL;
+	if (tx_memcpy_offload) {
+		qp->tx_offload_thread = kthread_run(ntb_tx_memcpy_kthread, qp,
+						    "ntb-txcpy/%s/%u",
+						    pci_name(ndev->pdev), qp->qp_num);
+		if (IS_ERR(qp->tx_offload_thread)) {
+			dev_warn(&nt->ndev->dev,
+				 "tx memcpy offload thread creation failed: %ld; falling back to inline copy\n",
+				 PTR_ERR(qp->tx_offload_thread));
+			qp->tx_offload_thread = NULL;
+		}
+	}
+
 	ntb_db_clear(qp->ndev, qp_bit);
 	ntb_db_clear_mask(qp->ndev, qp_bit);
 
-- 
2.51.0

Re: [PATCH v2 02/14] NTB: ntb_transport: Start TX offload thread after queue setup

[Frank Li]

On Thu, Sep 10, 2026 at 01:08:24PM +0900, Koichiro Den wrote:
> ntb_transport_create_queue() starts the per-QP TX offload thread before
> DMA mappings and queue entries are allocated. If later setup fails, the
> error path returns the QP to the free bitmap without stopping the
> thread. A retry can then reinitialize its waitqueue while the old thread
> is still waiting on it.
>
> Start the thread after queue setup.
>
> Fixes: 322617a06c97 ("NTB: ntb_transport: Add 'tx_memcpy_offload' module option")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v2:
>   - No changes.
>
> NOTE: Originally submitted as part of the direct TX/RX series v1:
> https://lore.kernel.org/r/20260810165136.2292436-4-den@valinux.co.jp/
>
>  drivers/ntb/ntb_transport.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 3389d6ca9ebd..55a20ae9a85e 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -2055,20 +2055,6 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
>  	qp->tx_handler = handlers->tx_handler;
>  	qp->event_handler = handlers->event_handler;
>
> -	init_waitqueue_head(&qp->tx_offload_wq);
> -	if (tx_memcpy_offload) {
> -		qp->tx_offload_thread = kthread_run(ntb_tx_memcpy_kthread, qp,
> -						    "ntb-txcpy/%s/%u",
> -						    pci_name(ndev->pdev), qp->qp_num);
> -		if (IS_ERR(qp->tx_offload_thread)) {
> -			dev_warn(&nt->ndev->dev,
> -				 "tx memcpy offload thread creation failed: %ld; falling back to inline copy\n",
> -				 PTR_ERR(qp->tx_offload_thread));
> -			qp->tx_offload_thread = NULL;
> -		}
> -	} else
> -		qp->tx_offload_thread = NULL;
> -
>  	dma_cap_zero(dma_mask);
>  	dma_cap_set(DMA_MEMCPY, dma_mask);
>
> @@ -2129,6 +2115,20 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
>  			     &qp->tx_free_q);
>  	}
>
> +	init_waitqueue_head(&qp->tx_offload_wq);
> +	qp->tx_offload_thread = NULL;
> +	if (tx_memcpy_offload) {
> +		qp->tx_offload_thread = kthread_run(ntb_tx_memcpy_kthread, qp,
> +						    "ntb-txcpy/%s/%u",
> +						    pci_name(ndev->pdev), qp->qp_num);
> +		if (IS_ERR(qp->tx_offload_thread)) {
> +			dev_warn(&nt->ndev->dev,
> +				 "tx memcpy offload thread creation failed: %ld; falling back to inline copy\n",
> +				 PTR_ERR(qp->tx_offload_thread));
> +			qp->tx_offload_thread = NULL;
> +		}
> +	}
> +
>  	ntb_db_clear(qp->ndev, qp_bit);
>  	ntb_db_clear_mask(qp->ndev, qp_bit);
>
> --
> 2.51.0
>

Re: [PATCH v2 02/14] NTB: ntb_transport: Start TX offload thread after queue setup

[Logan Gunthorpe]

On 2026-09-09 22:08, Koichiro Den wrote:
> ntb_transport_create_queue() starts the per-QP TX offload thread before
> DMA mappings and queue entries are allocated. If later setup fails, the
> error path returns the QP to the free bitmap without stopping the
> thread. A retry can then reinitialize its waitqueue while the old thread
> is still waiting on it.
> 
> Start the thread after queue setup.
> 
> Fixes: 322617a06c97 ("NTB: ntb_transport: Add 'tx_memcpy_offload' module option")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 03/14] NTB: ntb_transport: Avoid deadlock when cancelling link work

[Koichiro Den]

During initial link setup, ntb_transport_link_work() can retry with
nt->link_is_up still false. A retry can block on link_event_lock
while cleanup holds it and waits in cancel_delayed_work_sync(),
leading to deadlock.

Move the conditional cancellation outside link_event_lock, before
QP cleanup. Keep QP cleanup and MW release under the lock so link
work cannot restart QPs between them. Put the locking in
ntb_transport_link_cleanup() to cover both worker and remove paths.

Fixes: 3db835dd8f9a ("ntb: Add mutex to make link_event_callback executed linearly.")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 55a20ae9a85e..c77b173dca01 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -960,6 +960,15 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
 	struct ntb_transport_qp *qp;
 	u64 qp_bitmap_alloc;
 	unsigned int i, count;
+	bool cancel_link_work;
+
+	scoped_guard(mutex, &nt->link_event_lock)
+		cancel_link_work = !nt->link_is_up;
+
+	if (cancel_link_work)
+		cancel_delayed_work_sync(&nt->link_work);
+
+	guard(mutex)(&nt->link_event_lock);
 
 	qp_bitmap_alloc = nt->qp_bitmap & ~nt->qp_bitmap_free;
 
@@ -972,9 +981,6 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
 			cancel_delayed_work_sync(&qp->link_work);
 		}
 
-	if (!nt->link_is_up)
-		cancel_delayed_work_sync(&nt->link_work);
-
 	for (i = 0; i < nt->mw_count; i++)
 		ntb_free_mw(nt, i);
 
@@ -992,7 +998,6 @@ static void ntb_transport_link_cleanup_work(struct work_struct *work)
 	struct ntb_transport_ctx *nt =
 		container_of(work, struct ntb_transport_ctx, link_cleanup);
 
-	guard(mutex)(&nt->link_event_lock);
 	ntb_transport_link_cleanup(nt);
 }
 
-- 
2.51.0

Re: [PATCH v2 03/14] NTB: ntb_transport: Avoid deadlock when cancelling link work

[Frank Li]

On Thu, Sep 10, 2026 at 01:08:25PM +0900, Koichiro Den wrote:
> During initial link setup, ntb_transport_link_work() can retry with
> nt->link_is_up still false. A retry can block on link_event_lock
> while cleanup holds it and waits in cancel_delayed_work_sync(),
> leading to deadlock.
>
> Move the conditional cancellation outside link_event_lock, before
> QP cleanup. Keep QP cleanup and MW release under the lock so link
> work cannot restart QPs between them. Put the locking in
> ntb_transport_link_cleanup() to cover both worker and remove paths.
>
> Fixes: 3db835dd8f9a ("ntb: Add mutex to make link_event_callback executed linearly.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v2:
>   - No changes.
>
>  drivers/ntb/ntb_transport.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 55a20ae9a85e..c77b173dca01 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -960,6 +960,15 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
>  	struct ntb_transport_qp *qp;
>  	u64 qp_bitmap_alloc;
>  	unsigned int i, count;
> +	bool cancel_link_work;
> +
> +	scoped_guard(mutex, &nt->link_event_lock)
> +		cancel_link_work = !nt->link_is_up;
> +
> +	if (cancel_link_work)
> +		cancel_delayed_work_sync(&nt->link_work);

what happen if schedule new work here?

Frank
> +
> +	guard(mutex)(&nt->link_event_lock);
>
>  	qp_bitmap_alloc = nt->qp_bitmap & ~nt->qp_bitmap_free;
>
> @@ -972,9 +981,6 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
>  			cancel_delayed_work_sync(&qp->link_work);
>  		}
>
> -	if (!nt->link_is_up)
> -		cancel_delayed_work_sync(&nt->link_work);
> -
>  	for (i = 0; i < nt->mw_count; i++)
>  		ntb_free_mw(nt, i);
>
> @@ -992,7 +998,6 @@ static void ntb_transport_link_cleanup_work(struct work_struct *work)
>  	struct ntb_transport_ctx *nt =
>  		container_of(work, struct ntb_transport_ctx, link_cleanup);
>
> -	guard(mutex)(&nt->link_event_lock);
>  	ntb_transport_link_cleanup(nt);
>  }
>
> --
> 2.51.0
>

Re: [PATCH v2 03/14] NTB: ntb_transport: Avoid deadlock when cancelling link work

[Koichiro Den]

On Fri, Sep 11, 2026 at 11:21:43AM -0500, Frank Li wrote:
> On Thu, Sep 10, 2026 at 01:08:25PM +0900, Koichiro Den wrote:
> > During initial link setup, ntb_transport_link_work() can retry with
> > nt->link_is_up still false. A retry can block on link_event_lock
> > while cleanup holds it and waits in cancel_delayed_work_sync(),
> > leading to deadlock.
> >
> > Move the conditional cancellation outside link_event_lock, before
> > QP cleanup. Keep QP cleanup and MW release under the lock so link
> > work cannot restart QPs between them. Put the locking in
> > ntb_transport_link_cleanup() to cover both worker and remove paths.
> >
> > Fixes: 3db835dd8f9a ("ntb: Add mutex to make link_event_callback executed linearly.")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> > Changes in v2:
> >   - No changes.
> >
> >  drivers/ntb/ntb_transport.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 55a20ae9a85e..c77b173dca01 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -960,6 +960,15 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
> >  	struct ntb_transport_qp *qp;
> >  	u64 qp_bitmap_alloc;
> >  	unsigned int i, count;
> > +	bool cancel_link_work;
> > +
> > +	scoped_guard(mutex, &nt->link_event_lock)
> > +		cancel_link_work = !nt->link_is_up;
> > +
> > +	if (cancel_link_work)
> > +		cancel_delayed_work_sync(&nt->link_work);
> 
> what happen if schedule new work here?

Both link_work and link_cleanup can race to take link_event_lock.

- If link_work wins and completes setup, cleanup can tear the link down again.
  The old code also allowed setup to run before a pending cleanup though. In
  that sense, although Sashiko flagged it as a new issue, I believe the ordering
  issue itself is pre-existing.
- If link_cleanup wins, link_work just waits for cleanup to finish before
  starting setup.

This patch fixes the deadlock, not the event ordering. I would prefer to handle
the ordering issue separately.

Best regards,
Koichiro

> 
> Frank
> > +
> > +	guard(mutex)(&nt->link_event_lock);
> >
> >  	qp_bitmap_alloc = nt->qp_bitmap & ~nt->qp_bitmap_free;
> >
> > @@ -972,9 +981,6 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
> >  			cancel_delayed_work_sync(&qp->link_work);
> >  		}
> >
> > -	if (!nt->link_is_up)
> > -		cancel_delayed_work_sync(&nt->link_work);
> > -
> >  	for (i = 0; i < nt->mw_count; i++)
> >  		ntb_free_mw(nt, i);
> >
> > @@ -992,7 +998,6 @@ static void ntb_transport_link_cleanup_work(struct work_struct *work)
> >  	struct ntb_transport_ctx *nt =
> >  		container_of(work, struct ntb_transport_ctx, link_cleanup);
> >
> > -	guard(mutex)(&nt->link_event_lock);
> >  	ntb_transport_link_cleanup(nt);
> >  }
> >
> > --
> > 2.51.0
> >

Re: [PATCH v2 03/14] NTB: ntb_transport: Avoid deadlock when cancelling link work

[Logan Gunthorpe]

On 2026-09-09 22:08, Koichiro Den wrote:
> During initial link setup, ntb_transport_link_work() can retry with
> nt->link_is_up still false. A retry can block on link_event_lock
> while cleanup holds it and waits in cancel_delayed_work_sync(),
> leading to deadlock.
> 
> Move the conditional cancellation outside link_event_lock, before
> QP cleanup. Keep QP cleanup and MW release under the lock so link
> work cannot restart QPs between them. Put the locking in
> ntb_transport_link_cleanup() to cover both worker and remove paths.
> 
> Fixes: 3db835dd8f9a ("ntb: Add mutex to make link_event_callback executed linearly.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
Even with the ordering issue discussed, I think this is an improvement
worth doing:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 04/14] NTB: ntb_transport: Publish link state after QP setup

[Koichiro Den]

ntb_transport_link_work() marks the transport link up before setting
up the QPs' MW and peer MSI state. A concurrent ntb_transport_link_up()
can then queue QP link work, which may enable RX and notify the client
before setup finishes.

Publish link_is_up with a release store after setting up all QPs,
and use acquire loads before queuing QP link work.

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/anyKbq3mpLG4y7rb@SMW015318
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 40 +++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index c77b173dca01..75d4a2e021f4 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -922,6 +922,16 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
 		qp->remote_rx_info->entry = qp->rx_max_entry - 1;
 }
 
+static void ntb_transport_schedule_qp_link(struct ntb_transport_qp *qp,
+					   unsigned long delay)
+{
+	struct ntb_transport_ctx *nt = qp->transport;
+
+	/* Pair with the link publication in ntb_transport_link_work(). */
+	if (smp_load_acquire(&nt->link_is_up))
+		schedule_delayed_work(&qp->link_work, delay);
+}
+
 static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
 {
 	struct ntb_transport_ctx *nt = qp->transport;
@@ -941,13 +951,10 @@ static void ntb_qp_link_cleanup_work(struct work_struct *work)
 	struct ntb_transport_qp *qp = container_of(work,
 						   struct ntb_transport_qp,
 						   link_cleanup);
-	struct ntb_transport_ctx *nt = qp->transport;
 
 	ntb_qp_link_cleanup(qp);
-
-	if (nt->link_is_up)
-		schedule_delayed_work(&qp->link_work,
-				      msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
+	ntb_transport_schedule_qp_link(qp,
+				       msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
 }
 
 static void ntb_qp_link_down(struct ntb_transport_qp *qp)
@@ -1089,16 +1096,19 @@ static void ntb_transport_link_work(struct work_struct *work)
 			goto out1;
 	}
 
-	nt->link_is_up = true;
-
 	for (i = 0; i < nt->qp_count; i++) {
-		struct ntb_transport_qp *qp = &nt->qp_vec[i];
-
 		ntb_transport_setup_qp_mw(nt, i);
 		ntb_transport_setup_qp_peer_msi(nt, i);
+	}
+
+	/* Publish the link only after every QP has been set up. */
+	smp_store_release(&nt->link_is_up, true);
+
+	for (i = 0; i < nt->qp_count; i++) {
+		struct ntb_transport_qp *qp = &nt->qp_vec[i];
 
 		if (qp->client_ready)
-			schedule_delayed_work(&qp->link_work, 0);
+			ntb_transport_schedule_qp_link(qp, 0);
 	}
 
 	return;
@@ -1146,9 +1156,10 @@ static void ntb_qp_link_work(struct work_struct *work)
 
 		if (qp->active)
 			tasklet_schedule(&qp->rxc_db_work);
-	} else if (nt->link_is_up)
-		schedule_delayed_work(&qp->link_work,
-				      msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
+	} else {
+		ntb_transport_schedule_qp_link(qp,
+					       msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
+	}
 }
 
 static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
@@ -2392,8 +2403,7 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
 
 	qp->client_ready = true;
 
-	if (qp->transport->link_is_up)
-		schedule_delayed_work(&qp->link_work, 0);
+	ntb_transport_schedule_qp_link(qp, 0);
 }
 EXPORT_SYMBOL_GPL(ntb_transport_link_up);
 
-- 
2.51.0

Re: [PATCH v2 04/14] NTB: ntb_transport: Publish link state after QP setup

[Frank Li]

On Thu, Sep 10, 2026 at 01:08:26PM +0900, Koichiro Den wrote:
> ntb_transport_link_work() marks the transport link up before setting
> up the QPs' MW and peer MSI state. A concurrent ntb_transport_link_up()
> can then queue QP link work, which may enable RX and notify the client
> before setup finishes.
>
> Publish link_is_up with a release store after setting up all QPs,
> and use acquire loads before queuing QP link work.
>
> Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/anyKbq3mpLG4y7rb@SMW015318
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v2:
>   - No changes.
>
>  drivers/ntb/ntb_transport.c | 40 +++++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index c77b173dca01..75d4a2e021f4 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -922,6 +922,16 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
>  		qp->remote_rx_info->entry = qp->rx_max_entry - 1;
>  }
>
> +static void ntb_transport_schedule_qp_link(struct ntb_transport_qp *qp,
> +					   unsigned long delay)
> +{
> +	struct ntb_transport_ctx *nt = qp->transport;
> +
> +	/* Pair with the link publication in ntb_transport_link_work(). */
> +	if (smp_load_acquire(&nt->link_is_up))
> +		schedule_delayed_work(&qp->link_work, delay);
> +}
> +
>  static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
>  {
>  	struct ntb_transport_ctx *nt = qp->transport;
> @@ -941,13 +951,10 @@ static void ntb_qp_link_cleanup_work(struct work_struct *work)
>  	struct ntb_transport_qp *qp = container_of(work,
>  						   struct ntb_transport_qp,
>  						   link_cleanup);
> -	struct ntb_transport_ctx *nt = qp->transport;
>
>  	ntb_qp_link_cleanup(qp);
> -
> -	if (nt->link_is_up)
> -		schedule_delayed_work(&qp->link_work,
> -				      msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
> +	ntb_transport_schedule_qp_link(qp,
> +				       msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
>  }
>
>  static void ntb_qp_link_down(struct ntb_transport_qp *qp)
> @@ -1089,16 +1096,19 @@ static void ntb_transport_link_work(struct work_struct *work)
>  			goto out1;
>  	}
>
> -	nt->link_is_up = true;
> -
>  	for (i = 0; i < nt->qp_count; i++) {
> -		struct ntb_transport_qp *qp = &nt->qp_vec[i];
> -
>  		ntb_transport_setup_qp_mw(nt, i);
>  		ntb_transport_setup_qp_peer_msi(nt, i);
> +	}
> +
> +	/* Publish the link only after every QP has been set up. */
> +	smp_store_release(&nt->link_is_up, true);
> +
> +	for (i = 0; i < nt->qp_count; i++) {
> +		struct ntb_transport_qp *qp = &nt->qp_vec[i];
>
>  		if (qp->client_ready)
> -			schedule_delayed_work(&qp->link_work, 0);
> +			ntb_transport_schedule_qp_link(qp, 0);
>  	}
>
>  	return;
> @@ -1146,9 +1156,10 @@ static void ntb_qp_link_work(struct work_struct *work)
>
>  		if (qp->active)
>  			tasklet_schedule(&qp->rxc_db_work);
> -	} else if (nt->link_is_up)
> -		schedule_delayed_work(&qp->link_work,
> -				      msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
> +	} else {
> +		ntb_transport_schedule_qp_link(qp,
> +					       msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
> +	}
>  }
>
>  static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
> @@ -2392,8 +2403,7 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
>
>  	qp->client_ready = true;
>
> -	if (qp->transport->link_is_up)
> -		schedule_delayed_work(&qp->link_work, 0);
> +	ntb_transport_schedule_qp_link(qp, 0);
>  }
>  EXPORT_SYMBOL_GPL(ntb_transport_link_up);
>
> --
> 2.51.0
>

Re: [PATCH v2 04/14] NTB: ntb_transport: Publish link state after QP setup

[Logan Gunthorpe]

On 2026-09-09 22:08, Koichiro Den wrote:
> ntb_transport_link_work() marks the transport link up before setting
> up the QPs' MW and peer MSI state. A concurrent ntb_transport_link_up()
> can then queue QP link work, which may enable RX and notify the client
> before setup finishes.
> 
> Publish link_is_up with a release store after setting up all QPs,
> and use acquire loads before queuing QP link work.
> 
> Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/anyKbq3mpLG4y7rb@SMW015318
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 05/14] NTB: ntb_transport: Avoid losing QP link-up requests

[Koichiro Den]

ntb_netdev_open() can call ntb_transport_link_up() while the transport
worker is completing setup on another CPU. Concurrent transport setup
and a client link-up request can both read the other's flag as false and
leave QP link work unqueued. The QP then stays down until another link
event or client link-up request.

This is the store-buffering pattern described in
tools/memory-model/Documentation/recipes.txt ("Store buffering").

Add a full barrier between the store and load on each side, and
mark the client_ready accesses with READ_ONCE()/WRITE_ONCE().

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@smtp.kernel.org/
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - New patch (Sashiko)

 drivers/ntb/ntb_transport.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 75d4a2e021f4..1332d53bcfe7 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1104,10 +1104,16 @@ static void ntb_transport_link_work(struct work_struct *work)
 	/* Publish the link only after every QP has been set up. */
 	smp_store_release(&nt->link_is_up, true);
 
+	/*
+	 * Prevent both sides from missing each other's flag. Pairs with
+	 * the barrier in ntb_transport_link_up().
+	 */
+	smp_mb();
+
 	for (i = 0; i < nt->qp_count; i++) {
 		struct ntb_transport_qp *qp = &nt->qp_vec[i];
 
-		if (qp->client_ready)
+		if (READ_ONCE(qp->client_ready))
 			ntb_transport_schedule_qp_link(qp, 0);
 	}
 
@@ -2401,7 +2407,10 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
 	if (!qp)
 		return;
 
-	qp->client_ready = true;
+	WRITE_ONCE(qp->client_ready, true);
+
+	/* Pairs with the barrier in ntb_transport_link_work(). */
+	smp_mb();
 
 	ntb_transport_schedule_qp_link(qp, 0);
 }
-- 
2.51.0

Re: [PATCH v2 05/14] NTB: ntb_transport: Avoid losing QP link-up requests

[Frank Li]

On Thu, Sep 10, 2026 at 01:08:27PM +0900, Koichiro Den wrote:
> ntb_netdev_open() can call ntb_transport_link_up() while the transport
> worker is completing setup on another CPU. Concurrent transport setup
> and a client link-up request can both read the other's flag as false and
> leave QP link work unqueued. The QP then stays down until another link
> event or client link-up request.
>
> This is the store-buffering pattern described in
> tools/memory-model/Documentation/recipes.txt ("Store buffering").
>
> Add a full barrier between the store and load on each side, and
> mark the client_ready accesses with READ_ONCE()/WRITE_ONCE().
>
> Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@smtp.kernel.org/
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v2:
>   - New patch (Sashiko)
>
>  drivers/ntb/ntb_transport.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 75d4a2e021f4..1332d53bcfe7 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1104,10 +1104,16 @@ static void ntb_transport_link_work(struct work_struct *work)
>  	/* Publish the link only after every QP has been set up. */
>  	smp_store_release(&nt->link_is_up, true);
>
> +	/*
> +	 * Prevent both sides from missing each other's flag. Pairs with
> +	 * the barrier in ntb_transport_link_up().
> +	 */
> +	smp_mb();
> +
>  	for (i = 0; i < nt->qp_count; i++) {
>  		struct ntb_transport_qp *qp = &nt->qp_vec[i];
>
> -		if (qp->client_ready)
> +		if (READ_ONCE(qp->client_ready))

I think it'd better change to use atomic variable for client_ready to avoid
manual handle smp sync.

Frank

>  			ntb_transport_schedule_qp_link(qp, 0);
>  	}
>
> @@ -2401,7 +2407,10 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
>  	if (!qp)
>  		return;
>
> -	qp->client_ready = true;
> +	WRITE_ONCE(qp->client_ready, true);
> +
> +	/* Pairs with the barrier in ntb_transport_link_work(). */
> +	smp_mb();
>
>  	ntb_transport_schedule_qp_link(qp, 0);
>  }
> --
> 2.51.0
>

Re: [PATCH v2 05/14] NTB: ntb_transport: Avoid losing QP link-up requests

[Koichiro Den]

On Fri, Sep 11, 2026 at 11:53:37AM -0500, Frank Li wrote:
> On Thu, Sep 10, 2026 at 01:08:27PM +0900, Koichiro Den wrote:
> > ntb_netdev_open() can call ntb_transport_link_up() while the transport
> > worker is completing setup on another CPU. Concurrent transport setup
> > and a client link-up request can both read the other's flag as false and
> > leave QP link work unqueued. The QP then stays down until another link
> > event or client link-up request.
> >
> > This is the store-buffering pattern described in
> > tools/memory-model/Documentation/recipes.txt ("Store buffering").
> >
> > Add a full barrier between the store and load on each side, and
> > mark the client_ready accesses with READ_ONCE()/WRITE_ONCE().
> >
> > Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> > Cc: stable@vger.kernel.org
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@smtp.kernel.org/
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> > Changes in v2:
> >   - New patch (Sashiko)
> >
> >  drivers/ntb/ntb_transport.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 75d4a2e021f4..1332d53bcfe7 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -1104,10 +1104,16 @@ static void ntb_transport_link_work(struct work_struct *work)
> >  	/* Publish the link only after every QP has been set up. */
> >  	smp_store_release(&nt->link_is_up, true);
> >
> > +	/*
> > +	 * Prevent both sides from missing each other's flag. Pairs with
> > +	 * the barrier in ntb_transport_link_up().
> > +	 */
> > +	smp_mb();
> > +
> >  	for (i = 0; i < nt->qp_count; i++) {
> >  		struct ntb_transport_qp *qp = &nt->qp_vec[i];
> >
> > -		if (qp->client_ready)
> > +		if (READ_ONCE(qp->client_ready))
> 
> I think it'd better change to use atomic variable for client_ready to avoid
> manual handle smp sync.

AFAICT converting client_ready to atomic_t and using atomic_set()/atomic_read()
instead of WRITE_ONCE()/READ_ONCE() would not free us from the smp sync, as
those operations are unordered.
So to illustrate this in litmus test like form:

   # L = nt->link_is_up
   # R = qp->client_ready
   # Both initially false

   Transport setup                  Client link-up
   ---------------                  --------------
    smp_store_release(&L, true);     WRITE_ONCE(R, true);
    smp_mb(); /* added */            smp_mb(); /* added */
    r0 = READ_ONCE(R);               r1 = smp_load_acquire(&L);

   Both reads return false?
    Before: allowed
    After:  forbidden

I might be misunderstanding your suggestion though. If you have something
different in mind, please let me know.

Thanks for the review.
Koichiro

> 
> Frank
> 
> >  			ntb_transport_schedule_qp_link(qp, 0);
> >  	}
> >
> > @@ -2401,7 +2407,10 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
> >  	if (!qp)
> >  		return;
> >
> > -	qp->client_ready = true;
> > +	WRITE_ONCE(qp->client_ready, true);
> > +
> > +	/* Pairs with the barrier in ntb_transport_link_work(). */
> > +	smp_mb();
> >
> >  	ntb_transport_schedule_qp_link(qp, 0);
> >  }
> > --
> > 2.51.0
> >

Re: [PATCH v2 05/14] NTB: ntb_transport: Avoid losing QP link-up requests

[Koichiro Den]

On Sat, Sep 12, 2026 at 03:04:21AM +0900, Koichiro Den wrote:
> On Fri, Sep 11, 2026 at 11:53:37AM -0500, Frank Li wrote:
> > On Thu, Sep 10, 2026 at 01:08:27PM +0900, Koichiro Den wrote:
> > > ntb_netdev_open() can call ntb_transport_link_up() while the transport
> > > worker is completing setup on another CPU. Concurrent transport setup
> > > and a client link-up request can both read the other's flag as false and
> > > leave QP link work unqueued. The QP then stays down until another link
> > > event or client link-up request.
> > >
> > > This is the store-buffering pattern described in
> > > tools/memory-model/Documentation/recipes.txt ("Store buffering").
> > >
> > > Add a full barrier between the store and load on each side, and
> > > mark the client_ready accesses with READ_ONCE()/WRITE_ONCE().
> > >
> > > Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> > > Cc: stable@vger.kernel.org
> > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@smtp.kernel.org/
> > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > ---
> > > Changes in v2:
> > >   - New patch (Sashiko)
> > >
> > >  drivers/ntb/ntb_transport.c | 13 +++++++++++--
> > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > > index 75d4a2e021f4..1332d53bcfe7 100644
> > > --- a/drivers/ntb/ntb_transport.c
> > > +++ b/drivers/ntb/ntb_transport.c
> > > @@ -1104,10 +1104,16 @@ static void ntb_transport_link_work(struct work_struct *work)
> > >  	/* Publish the link only after every QP has been set up. */
> > >  	smp_store_release(&nt->link_is_up, true);
> > >
> > > +	/*
> > > +	 * Prevent both sides from missing each other's flag. Pairs with
> > > +	 * the barrier in ntb_transport_link_up().
> > > +	 */
> > > +	smp_mb();
> > > +
> > >  	for (i = 0; i < nt->qp_count; i++) {
> > >  		struct ntb_transport_qp *qp = &nt->qp_vec[i];
> > >
> > > -		if (qp->client_ready)
> > > +		if (READ_ONCE(qp->client_ready))
> > 
> > I think it'd better change to use atomic variable for client_ready to avoid
> > manual handle smp sync.
> 
> AFAICT converting client_ready to atomic_t and using atomic_set()/atomic_read()
> instead of WRITE_ONCE()/READ_ONCE() would not free us from the smp sync, as
> those operations are unordered.
> So to illustrate this in litmus test like form:
> 
>    # L = nt->link_is_up
>    # R = qp->client_ready
>    # Both initially false
> 
>    Transport setup                  Client link-up
>    ---------------                  --------------
>     smp_store_release(&L, true);     WRITE_ONCE(R, true);
>     smp_mb(); /* added */            smp_mb(); /* added */
>     r0 = READ_ONCE(R);               r1 = smp_load_acquire(&L);
> 
>    Both reads return false?
>     Before: allowed
>     After:  forbidden
> 
> I might be misunderstanding your suggestion though. If you have something
> different in mind, please let me know.

Just for the record, smp_store_release/smp_load_acquire depicted above, instead
of WRITE_ONCE()/READ_ONCE(), is intentional. They are for MP ordering to publish
the QP setup, which is needed by an earlier patch:
https://lore.kernel.org/r/20260910040836.3792333-5-den@valinux.co.jp/

Best regards,
Koichiro

> 
> Thanks for the review.
> Koichiro
> 
> > 
> > Frank
> > 
> > >  			ntb_transport_schedule_qp_link(qp, 0);
> > >  	}
> > >
> > > @@ -2401,7 +2407,10 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
> > >  	if (!qp)
> > >  		return;
> > >
> > > -	qp->client_ready = true;
> > > +	WRITE_ONCE(qp->client_ready, true);
> > > +
> > > +	/* Pairs with the barrier in ntb_transport_link_work(). */
> > > +	smp_mb();
> > >
> > >  	ntb_transport_schedule_qp_link(qp, 0);
> > >  }
> > > --
> > > 2.51.0
> > >

Re: [PATCH v2 05/14] NTB: ntb_transport: Avoid losing QP link-up requests

[Frank Li]

On Sat, Sep 12, 2026 at 03:21:08AM +0900, Koichiro Den wrote:
> On Sat, Sep 12, 2026 at 03:04:21AM +0900, Koichiro Den wrote:
> > On Fri, Sep 11, 2026 at 11:53:37AM -0500, Frank Li wrote:
> > > On Thu, Sep 10, 2026 at 01:08:27PM +0900, Koichiro Den wrote:
> > > > ntb_netdev_open() can call ntb_transport_link_up() while the transport
> > > > worker is completing setup on another CPU. Concurrent transport setup
> > > > and a client link-up request can both read the other's flag as false and
> > > > leave QP link work unqueued. The QP then stays down until another link
> > > > event or client link-up request.
> > > >
> > > > This is the store-buffering pattern described in
> > > > tools/memory-model/Documentation/recipes.txt ("Store buffering").
> > > >
> > > > Add a full barrier between the store and load on each side, and
> > > > mark the client_ready accesses with READ_ONCE()/WRITE_ONCE().
> > > >
> > > > Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> > > > Cc: stable@vger.kernel.org
> > > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > > Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@smtp.kernel.org/
> > > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > > ---
> > > > Changes in v2:
> > > >   - New patch (Sashiko)
> > > >
> > > >  drivers/ntb/ntb_transport.c | 13 +++++++++++--
> > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > > > index 75d4a2e021f4..1332d53bcfe7 100644
> > > > --- a/drivers/ntb/ntb_transport.c
> > > > +++ b/drivers/ntb/ntb_transport.c
> > > > @@ -1104,10 +1104,16 @@ static void ntb_transport_link_work(struct work_struct *work)
> > > >  	/* Publish the link only after every QP has been set up. */
> > > >  	smp_store_release(&nt->link_is_up, true);
> > > >
> > > > +	/*
> > > > +	 * Prevent both sides from missing each other's flag. Pairs with
> > > > +	 * the barrier in ntb_transport_link_up().
> > > > +	 */
> > > > +	smp_mb();
> > > > +
> > > >  	for (i = 0; i < nt->qp_count; i++) {
> > > >  		struct ntb_transport_qp *qp = &nt->qp_vec[i];
> > > >
> > > > -		if (qp->client_ready)
> > > > +		if (READ_ONCE(qp->client_ready))
> > >
> > > I think it'd better change to use atomic variable for client_ready to avoid
> > > manual handle smp sync.
> >
> > AFAICT converting client_ready to atomic_t and using atomic_set()/atomic_read()
> > instead of WRITE_ONCE()/READ_ONCE() would not free us from the smp sync, as
> > those operations are unordered.

there are acquire version for atomic

atomic_set_release()

atomic_read_acquire()

My key point is use existing higher level sync APIs to avoid consider
barrier problem, which need more brain cell to think it.

this is just sync state, which don't impact performance.

Frank

> > So to illustrate this in litmus test like form:
> >
> >    # L = nt->link_is_up
> >    # R = qp->client_ready
> >    # Both initially false
> >
> >    Transport setup                  Client link-up
> >    ---------------                  --------------
> >     smp_store_release(&L, true);     WRITE_ONCE(R, true);
> >     smp_mb(); /* added */            smp_mb(); /* added */
> >     r0 = READ_ONCE(R);               r1 = smp_load_acquire(&L);
> >
> >    Both reads return false?
> >     Before: allowed
> >     After:  forbidden
> >
> > I might be misunderstanding your suggestion though. If you have something
> > different in mind, please let me know.
>
> Just for the record, smp_store_release/smp_load_acquire depicted above, instead
> of WRITE_ONCE()/READ_ONCE(), is intentional. They are for MP ordering to publish
> the QP setup, which is needed by an earlier patch:
> https://lore.kernel.org/r/20260910040836.3792333-5-den@valinux.co.jp/
>
> Best regards,
> Koichiro
>
> >
> > Thanks for the review.
> > Koichiro
> >
> > >
> > > Frank
> > >
> > > >  			ntb_transport_schedule_qp_link(qp, 0);
> > > >  	}
> > > >
> > > > @@ -2401,7 +2407,10 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
> > > >  	if (!qp)
> > > >  		return;
> > > >
> > > > -	qp->client_ready = true;
> > > > +	WRITE_ONCE(qp->client_ready, true);
> > > > +
> > > > +	/* Pairs with the barrier in ntb_transport_link_work(). */
> > > > +	smp_mb();
> > > >
> > > >  	ntb_transport_schedule_qp_link(qp, 0);
> > > >  }
> > > > --
> > > > 2.51.0
> > > >

Re: [PATCH v2 05/14] NTB: ntb_transport: Avoid losing QP link-up requests

[Koichiro Den]

On Fri, Sep 11, 2026 at 10:20:34PM -0500, Frank Li wrote:
> On Sat, Sep 12, 2026 at 03:21:08AM +0900, Koichiro Den wrote:
> > On Sat, Sep 12, 2026 at 03:04:21AM +0900, Koichiro Den wrote:
> > > On Fri, Sep 11, 2026 at 11:53:37AM -0500, Frank Li wrote:
> > > > On Thu, Sep 10, 2026 at 01:08:27PM +0900, Koichiro Den wrote:
> > > > > ntb_netdev_open() can call ntb_transport_link_up() while the transport
> > > > > worker is completing setup on another CPU. Concurrent transport setup
> > > > > and a client link-up request can both read the other's flag as false and
> > > > > leave QP link work unqueued. The QP then stays down until another link
> > > > > event or client link-up request.
> > > > >
> > > > > This is the store-buffering pattern described in
> > > > > tools/memory-model/Documentation/recipes.txt ("Store buffering").
> > > > >
> > > > > Add a full barrier between the store and load on each side, and
> > > > > mark the client_ready accesses with READ_ONCE()/WRITE_ONCE().
> > > > >
> > > > > Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> > > > > Cc: stable@vger.kernel.org
> > > > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > > > Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@smtp.kernel.org/
> > > > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > > > ---
> > > > > Changes in v2:
> > > > >   - New patch (Sashiko)
> > > > >
> > > > >  drivers/ntb/ntb_transport.c | 13 +++++++++++--
> > > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > > > > index 75d4a2e021f4..1332d53bcfe7 100644
> > > > > --- a/drivers/ntb/ntb_transport.c
> > > > > +++ b/drivers/ntb/ntb_transport.c
> > > > > @@ -1104,10 +1104,16 @@ static void ntb_transport_link_work(struct work_struct *work)
> > > > >  	/* Publish the link only after every QP has been set up. */
> > > > >  	smp_store_release(&nt->link_is_up, true);
> > > > >
> > > > > +	/*
> > > > > +	 * Prevent both sides from missing each other's flag. Pairs with
> > > > > +	 * the barrier in ntb_transport_link_up().
> > > > > +	 */
> > > > > +	smp_mb();
> > > > > +
> > > > >  	for (i = 0; i < nt->qp_count; i++) {
> > > > >  		struct ntb_transport_qp *qp = &nt->qp_vec[i];
> > > > >
> > > > > -		if (qp->client_ready)
> > > > > +		if (READ_ONCE(qp->client_ready))
> > > >
> > > > I think it'd better change to use atomic variable for client_ready to avoid
> > > > manual handle smp sync.
> > >
> > > AFAICT converting client_ready to atomic_t and using atomic_set()/atomic_read()
> > > instead of WRITE_ONCE()/READ_ONCE() would not free us from the smp sync, as
> > > those operations are unordered.
> 
> there are acquire version for atomic
> 
> atomic_set_release()
> 
> atomic_read_acquire()
> 
> My key point is use existing higher level sync APIs to avoid consider
> barrier problem, which need more brain cell to think it.
> 
> this is just sync state, which don't impact performance.

I agree with the idea of changing both flags to atomic_t for cleaner code, but
I'm not sure if it frees us from "the smp sync" or the "barrier problem". We'd
still need both smp_mb()s and still have to think about *_release/*_acquire when
revisiting the code and maybe scratching our heads, right?

BTW, I once wondered whether it might be worth consolidating
link_work/link_cleanup into a single state machine worker per transport or QP.
That could simplify synchronization between setup and cleanup, which aren't
really performance-critical, as you say. However, that would possibly require a
larger rework, so for now I've put together this fix series as a small step. I
would be interested to hear what NTB maintainers think about the possibility of
such a rework.

Best regards,
Koichiro

> 
> Frank
> 
> > > So to illustrate this in litmus test like form:
> > >
> > >    # L = nt->link_is_up
> > >    # R = qp->client_ready
> > >    # Both initially false
> > >
> > >    Transport setup                  Client link-up
> > >    ---------------                  --------------
> > >     smp_store_release(&L, true);     WRITE_ONCE(R, true);
> > >     smp_mb(); /* added */            smp_mb(); /* added */
> > >     r0 = READ_ONCE(R);               r1 = smp_load_acquire(&L);
> > >
> > >    Both reads return false?
> > >     Before: allowed
> > >     After:  forbidden
> > >
> > > I might be misunderstanding your suggestion though. If you have something
> > > different in mind, please let me know.
> >
> > Just for the record, smp_store_release/smp_load_acquire depicted above, instead
> > of WRITE_ONCE()/READ_ONCE(), is intentional. They are for MP ordering to publish
> > the QP setup, which is needed by an earlier patch:
> > https://lore.kernel.org/r/20260910040836.3792333-5-den@valinux.co.jp/
> >
> > Best regards,
> > Koichiro
> >
> > >
> > > Thanks for the review.
> > > Koichiro
> > >
> > > >
> > > > Frank
> > > >
> > > > >  			ntb_transport_schedule_qp_link(qp, 0);
> > > > >  	}
> > > > >
> > > > > @@ -2401,7 +2407,10 @@ void ntb_transport_link_up(struct ntb_transport_qp *qp)
> > > > >  	if (!qp)
> > > > >  		return;
> > > > >
> > > > > -	qp->client_ready = true;
> > > > > +	WRITE_ONCE(qp->client_ready, true);
> > > > > +
> > > > > +	/* Pairs with the barrier in ntb_transport_link_work(). */
> > > > > +	smp_mb();
> > > > >
> > > > >  	ntb_transport_schedule_qp_link(qp, 0);
> > > > >  }
> > > > > --
> > > > > 2.51.0
> > > > >

[PATCH v2 06/14] NTB: ntb_transport: Clear link state before QP cleanup

[Koichiro Den]

Cleanup leaves the transport link marked up after releasing its MWs.
A subsequent client link-up request can therefore start QP link work
before the transport has been set up again.

Clear link_is_up before cancelling QP work and releasing the MWs.
Have QP link work return if the transport went down after it was
queued.

Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

Note: this is a reworked version of my earlier, withdrawn patch:
https://lore.kernel.org/r/20260717061223.2203863-1-den@valinux.co.jp/

 drivers/ntb/ntb_transport.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 1332d53bcfe7..8dd1770aaaf1 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -977,6 +977,8 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
 
 	guard(mutex)(&nt->link_event_lock);
 
+	WRITE_ONCE(nt->link_is_up, false);
+
 	qp_bitmap_alloc = nt->qp_bitmap & ~nt->qp_bitmap_free;
 
 	/* Pass along the info to any clients */
@@ -1142,7 +1144,9 @@ static void ntb_qp_link_work(struct work_struct *work)
 	struct ntb_transport_ctx *nt = qp->transport;
 	int val;
 
-	WARN_ON(!nt->link_is_up);
+	/* Pair with the link publication in ntb_transport_link_work(). */
+	if (!smp_load_acquire(&nt->link_is_up))
+		return;
 
 	val = ntb_spad_read(nt->ndev, QP_LINKS);
 
-- 
2.51.0

Re: [PATCH v2 06/14] NTB: ntb_transport: Clear link state before QP cleanup

[Logan Gunthorpe]

On 2026-09-09 22:08, Koichiro Den wrote:
> Cleanup leaves the transport link marked up after releasing its MWs.
> A subsequent client link-up request can therefore start QP link work
> before the transport has been set up again.
> 
> Clear link_is_up before cancelling QP work and releasing the MWs.
> Have QP link work return if the transport went down after it was
> queued.
> 
> Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

This makes sense to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 07/14] NTB: ntb_transport: Stop QP work before freeing a queue

[Koichiro Den]

ntb_transport_free_queue() cancels qp->link_work but not qp->link_cleanup.
A peer link-down message can queue cleanup while ntb_netdev is freeing
the QP. Cleanup can then requeue link work after the queue resources
have been freed.

Disable and wait for cleanup, then link work, before freeing resources.
Unlike cancel, disable also prevents the RX tasklet and transport link
setup from queuing more work. Enable the works only after queue creation
succeeds.

Clear client_ready first so RX completions and transport link setup see
that the client is no longer ready. Clear link_is_up and active after
the workers stop, since link work can set both back to true.

Fixes: 7b4f2d3c3b82 ("NTB: No sleeping in interrupt context")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

Reworked from the corresponding patch in the direct TX/RX series v1:
https://lore.kernel.org/r/20260810165136.2292436-5-den@valinux.co.jp/

 drivers/ntb/ntb_transport.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 8dd1770aaaf1..e5599c7ca93f 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1242,6 +1242,8 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
 
 	INIT_DELAYED_WORK(&qp->link_work, ntb_qp_link_work);
 	INIT_WORK(&qp->link_cleanup, ntb_qp_link_cleanup_work);
+	disable_delayed_work(&qp->link_work);
+	disable_work(&qp->link_cleanup);
 
 	spin_lock_init(&qp->ntb_rx_q_lock);
 	spin_lock_init(&qp->ntb_tx_free_q_lock);
@@ -2155,6 +2157,9 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
 		}
 	}
 
+	enable_work(&qp->link_cleanup);
+	enable_delayed_work(&qp->link_work);
+
 	ntb_db_clear(qp->ndev, qp_bit);
 	ntb_db_clear_mask(qp->ndev, qp_bit);
 
@@ -2200,6 +2205,10 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)
 
 	pdev = qp->ndev->pdev;
 
+	qp->client_ready = false;
+	disable_work_sync(&qp->link_cleanup);
+	disable_delayed_work_sync(&qp->link_work);
+	qp->link_is_up = false;
 	qp->active = false;
 
 	if (qp->tx_offload_thread) {
@@ -2247,8 +2256,6 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)
 	ntb_db_set_mask(qp->ndev, qp_bit);
 	tasklet_kill(&qp->rxc_db_work);
 
-	cancel_delayed_work_sync(&qp->link_work);
-
 	qp->cb_data = NULL;
 	qp->rx_handler = NULL;
 	qp->tx_handler = NULL;
-- 
2.51.0

Re: [PATCH v2 07/14] NTB: ntb_transport: Stop QP work before freeing a queue

[Logan Gunthorpe]

On 2026-09-09 22:08, Koichiro Den wrote:
> ntb_transport_free_queue() cancels qp->link_work but not qp->link_cleanup.
> A peer link-down message can queue cleanup while ntb_netdev is freeing
> the QP. Cleanup can then requeue link work after the queue resources
> have been freed.
> 
> Disable and wait for cleanup, then link work, before freeing resources.
> Unlike cancel, disable also prevents the RX tasklet and transport link
> setup from queuing more work. Enable the works only after queue creation
> succeeds.
> 
> Clear client_ready first so RX completions and transport link setup see
> that the client is no longer ready. Clear link_is_up and active after
> the workers stop, since link work can set both back to true.
> 
> Fixes: 7b4f2d3c3b82 ("NTB: No sleeping in interrupt context")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Looks good to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 08/14] NTB: ntb_transport: Stop RX tasklet scheduling before freeing a queue

[Koichiro Den]

A caller can read qp->active before teardown clears it, then schedule
the RX tasklet after tasklet_kill() returns. The MSI handler does not
check active at all. Teardown also releases DMA channels before
draining the tasklet.

Protect active updates and the check-and-schedule sequence with
rx_sched_lock, including the MSI path. Clear active under the lock,
then drain the tasklet before releasing DMA channels or queue entries.
QP link work is already disabled, so it cannot reactivate RX. Use a
separate lock to avoid contention with RX list operations.

Fixes: e902133162af ("ntb: stop tasklet from spinning forever during shutdown.")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 50 +++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index e5599c7ca93f..45d4365becac 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -179,6 +179,8 @@ struct ntb_transport_qp {
 	unsigned int rx_max_frame;
 	unsigned int rx_alloc_entry;
 	dma_cookie_t last_cookie;
+	/* Protect active and RX tasklet scheduling. */
+	spinlock_t rx_sched_lock;
 	struct tasklet_struct rxc_db_work;
 
 	void (*event_handler)(void *data, int status);
@@ -649,11 +651,26 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
 	return 0;
 }
 
+static void ntb_transport_set_qp_active(struct ntb_transport_qp *qp, bool active)
+{
+	guard(spinlock_irqsave)(&qp->rx_sched_lock);
+
+	qp->active = active;
+}
+
+static void ntb_transport_schedule_rxc(struct ntb_transport_qp *qp)
+{
+	guard(spinlock_irqsave)(&qp->rx_sched_lock);
+
+	if (qp->active)
+		tasklet_schedule(&qp->rxc_db_work);
+}
+
 static irqreturn_t ntb_transport_isr(int irq, void *dev)
 {
 	struct ntb_transport_qp *qp = dev;
 
-	tasklet_schedule(&qp->rxc_db_work);
+	ntb_transport_schedule_rxc(qp);
 
 	return IRQ_HANDLED;
 }
@@ -895,7 +912,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
 static void ntb_qp_link_context_reset(struct ntb_transport_qp *qp)
 {
 	qp->link_is_up = false;
-	qp->active = false;
+	ntb_transport_set_qp_active(qp, false);
 
 	qp->tx_index = 0;
 	qp->rx_index = 0;
@@ -1159,13 +1176,12 @@ static void ntb_qp_link_work(struct work_struct *work)
 	if (val & BIT(qp->qp_num)) {
 		dev_info(&pdev->dev, "qp %d: Link Up\n", qp->qp_num);
 		qp->link_is_up = true;
-		qp->active = true;
+		ntb_transport_set_qp_active(qp, true);
 
 		if (qp->event_handler)
 			qp->event_handler(qp->cb_data, qp->link_is_up);
 
-		if (qp->active)
-			tasklet_schedule(&qp->rxc_db_work);
+		ntb_transport_schedule_rxc(qp);
 	} else {
 		ntb_transport_schedule_qp_link(qp,
 					       msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
@@ -1193,6 +1209,7 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
 	qp->ndev = nt->ndev;
 	qp->client_ready = false;
 	qp->event_handler = NULL;
+	spin_lock_init(&qp->rx_sched_lock);
 	ntb_qp_link_context_reset(qp);
 
 	if (mw_num < qp_count % mw_count)
@@ -1729,8 +1746,7 @@ static void ntb_transport_rxc_db(unsigned long data)
 
 	if (i == qp->rx_max_entry) {
 		/* there is more work to do */
-		if (qp->active)
-			tasklet_schedule(&qp->rxc_db_work);
+		ntb_transport_schedule_rxc(qp);
 	} else if (ntb_db_read(qp->ndev) & BIT_ULL(qp->qp_num)) {
 		/* the doorbell bit is set: clear it */
 		ntb_db_clear(qp->ndev, BIT_ULL(qp->qp_num));
@@ -1741,8 +1757,7 @@ static void ntb_transport_rxc_db(unsigned long data)
 		 * ntb_process_rxc and clearing the doorbell bit:
 		 * there might be some more work to do.
 		 */
-		if (qp->active)
-			tasklet_schedule(&qp->rxc_db_work);
+		ntb_transport_schedule_rxc(qp);
 	}
 }
 
@@ -2209,7 +2224,11 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)
 	disable_work_sync(&qp->link_cleanup);
 	disable_delayed_work_sync(&qp->link_work);
 	qp->link_is_up = false;
-	qp->active = false;
+	ntb_transport_set_qp_active(qp, false);
+
+	qp_bit = BIT_ULL(qp->qp_num);
+	ntb_db_set_mask(qp->ndev, qp_bit);
+	tasklet_kill(&qp->rxc_db_work);
 
 	if (qp->tx_offload_thread) {
 		kthread_stop(qp->tx_offload_thread);
@@ -2251,11 +2270,6 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)
 		dma_release_channel(chan);
 	}
 
-	qp_bit = BIT_ULL(qp->qp_num);
-
-	ntb_db_set_mask(qp->ndev, qp_bit);
-	tasklet_kill(&qp->rxc_db_work);
-
 	qp->cb_data = NULL;
 	qp->rx_handler = NULL;
 	qp->tx_handler = NULL;
@@ -2350,8 +2364,7 @@ int ntb_transport_rx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 
 	ntb_list_add(&qp->ntb_rx_q_lock, &entry->entry, &qp->rx_pend_q);
 
-	if (qp->active)
-		tasklet_schedule(&qp->rxc_db_work);
+	ntb_transport_schedule_rxc(qp);
 
 	return 0;
 }
@@ -2548,8 +2561,7 @@ static void ntb_transport_doorbell_callback(void *data, int vector)
 		qp_num = __ffs(db_bits);
 		qp = &nt->qp_vec[qp_num];
 
-		if (qp->active)
-			tasklet_schedule(&qp->rxc_db_work);
+		ntb_transport_schedule_rxc(qp);
 
 		db_bits &= ~BIT_ULL(qp_num);
 	}
-- 
2.51.0

[PATCH v2 09/14] NTB: ntb_transport: Drain RX tasklets during link cleanup

[Koichiro Den]

ntb_qp_link_cleanup() cancels QP link work but does not wait for the RX
tasklet. The tasklet can still be processing the ring while cleanup
resets the QP, and transport link cleanup can free the MW before the
tasklet finishes.

Clear active under rx_sched_lock and drain the tasklet before resetting
the QP. Temporarily disable QP link work so a concurrent client link-up
request cannot reactivate RX during cleanup, then re-enable it for the
existing link setup paths.

This does not drain RX DMA transfers or their completion callbacks.

Fixes: 9143595a7e05 ("NTB: ntb_transport: Free MWs in ntb_transport_link_cleanup()")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 45d4365becac..36797ea3ff45 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -956,11 +956,16 @@ static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
 
 	dev_info(&pdev->dev, "qp %d: Link Cleanup\n", qp->qp_num);
 
-	cancel_delayed_work_sync(&qp->link_work);
+	disable_delayed_work_sync(&qp->link_work);
+	ntb_transport_set_qp_active(qp, false);
+	tasklet_kill(&qp->rxc_db_work);
+
 	ntb_qp_link_down_reset(qp);
 
 	if (qp->event_handler)
 		qp->event_handler(qp->cb_data, qp->link_is_up);
+
+	enable_delayed_work(&qp->link_work);
 }
 
 static void ntb_qp_link_cleanup_work(struct work_struct *work)
-- 
2.51.0

[PATCH v2 10/14] NTB: ntb_transport: Wait for RX completions before resetting a QP

[Koichiro Den]

Transport link cleanup can free an MW still used by RX DMA or its
completion path. QP-only cleanup retains the MW, but can restart RX
on ring slots whose old completion callbacks have not yet cleared
the headers.

Wait for rx_post_q to empty before resetting the QP. ntb_complete_rxc()
finishes its MW accesses before removing each entry under ntb_rx_q_lock,
so cleanup can free the MW without racing with these RX accesses.
Using dmaengine_terminate_sync() instead would not work with drivers
such as IOAT that lack the required ops. Cookie-based waits would
not work with DMA_COMPLETION_NO_ORDER either.

DMA teardown in ntb_transport_free_queue() is unchanged.

Fixes: 9143595a7e05 ("NTB: ntb_transport: Free MWs in ntb_transport_link_cleanup()")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260907144257.767281F00A3A@smtp.kernel.org/
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - New patch (Sashiko).

 drivers/ntb/ntb_transport.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 36797ea3ff45..7ccba2c04f54 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -949,6 +949,13 @@ static void ntb_transport_schedule_qp_link(struct ntb_transport_qp *qp,
 		schedule_delayed_work(&qp->link_work, delay);
 }
 
+static bool ntb_transport_rx_idle(struct ntb_transport_qp *qp)
+{
+	guard(spinlock_irqsave)(&qp->ntb_rx_q_lock);
+
+	return list_empty(&qp->rx_post_q);
+}
+
 static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
 {
 	struct ntb_transport_ctx *nt = qp->transport;
@@ -959,6 +966,17 @@ static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
 	disable_delayed_work_sync(&qp->link_work);
 	ntb_transport_set_qp_active(qp, false);
 	tasklet_kill(&qp->rxc_db_work);
+	/*
+	 * Some DMA engines lack terminate/synchronize ops (e.g. IOAT), and
+	 * DMA_COMPLETION_NO_ORDER rules out cookie-based waits.
+	 *
+	 * Waiting for rx_post_q to empty suffices: ntb_complete_rxc() finishes
+	 * its MW accesses before removing each entry under ntb_rx_q_lock.
+	 * qp->active is false and rxc_db_work is stopped, so no new RX DMA
+	 * can be submitted.
+	 */
+	while (!ntb_transport_rx_idle(qp))
+		fsleep(1000);
 
 	ntb_qp_link_down_reset(qp);
 
-- 
2.51.0

[PATCH v2 11/14] NTB: ntb_transport: Prepare remote RX info accesses for MW teardown

[Koichiro Den]

The next patch clears remote_rx_info when freeing its MW.
ntb_transport_tx_free_entry() and debugfs stats reads can run during
link cleanup, so make them handle a NULL pointer.

The pointer is accessed locklessly. Use READ_ONCE() and WRITE_ONCE()
to prevent compiler-induced tearing, and retain the read value so
the NULL check and dereference use the same pointer.

Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 7ccba2c04f54..b949f36a4f2d 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -489,6 +489,7 @@ EXPORT_SYMBOL_GPL(ntb_transport_unregister_client);
 static int ntb_qp_debugfs_stats_show(struct seq_file *s, void *v)
 {
 	struct ntb_transport_qp *qp = s->private;
+	struct ntb_rx_info *remote_rx_info;
 
 	if (!qp || !qp->link_is_up)
 		return 0;
@@ -516,7 +517,9 @@ static int ntb_qp_debugfs_stats_show(struct seq_file *s, void *v)
 	seq_printf(s, "tx_err_no_buf - %llu\n", qp->tx_err_no_buf);
 	seq_printf(s, "tx_mw - \t0x%p\n", qp->tx_mw);
 	seq_printf(s, "tx_index (H) - \t%u\n", qp->tx_index);
-	seq_printf(s, "RRI (T) - \t%u\n", qp->remote_rx_info->entry);
+	remote_rx_info = READ_ONCE(qp->remote_rx_info);
+	if (remote_rx_info)
+		seq_printf(s, "RRI (T) - \t%u\n", remote_rx_info->entry);
 	seq_printf(s, "tx_max_entry - \t%u\n", qp->tx_max_entry);
 	seq_printf(s, "free tx - \t%u\n", ntb_transport_tx_free_entry(qp));
 	seq_putc(s, '\n');
@@ -611,7 +614,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
 	qp->rx_buff = mw->virt_addr + rx_size * (qp_num / mw_count);
 	rx_size -= sizeof(struct ntb_rx_info);
 
-	qp->remote_rx_info = qp->rx_buff + rx_size;
+	WRITE_ONCE(qp->remote_rx_info, qp->rx_buff + rx_size);
 
 	/* Due to housekeeping, there must be atleast 2 buffs */
 	qp->rx_max_frame = min(transport_mtu, rx_size / 2);
@@ -934,9 +937,12 @@ static void ntb_qp_link_context_reset(struct ntb_transport_qp *qp)
 
 static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
 {
+	struct ntb_rx_info *remote_rx_info;
+
 	ntb_qp_link_context_reset(qp);
-	if (qp->remote_rx_info)
-		qp->remote_rx_info->entry = qp->rx_max_entry - 1;
+	remote_rx_info = READ_ONCE(qp->remote_rx_info);
+	if (remote_rx_info)
+		remote_rx_info->entry = qp->rx_max_entry - 1;
 }
 
 static void ntb_transport_schedule_qp_link(struct ntb_transport_qp *qp,
@@ -2558,8 +2564,14 @@ EXPORT_SYMBOL_GPL(ntb_transport_max_size);
 
 unsigned int ntb_transport_tx_free_entry(struct ntb_transport_qp *qp)
 {
+	struct ntb_rx_info *remote_rx_info = READ_ONCE(qp->remote_rx_info);
 	unsigned int head = qp->tx_index;
-	unsigned int tail = qp->remote_rx_info->entry;
+	unsigned int tail;
+
+	if (!remote_rx_info)
+		return 0;
+
+	tail = remote_rx_info->entry;
 
 	return tail >= head ? tail - head : qp->tx_max_entry + tail - head;
 }
-- 
2.51.0

[PATCH v2 12/14] NTB: ntb_transport: Clear QP pointers when freeing an MW

[Koichiro Den]

ntb_transport_link_cleanup() frees MW buffers but leaves rx_buff and
remote_rx_info pointing into them. With a QP still allocated, another
link-down notification or transport unbind before MW setup runs again
can make ntb_qp_link_down_reset() write to freed memory through
remote_rx_info.

Clear both pointers in ntb_free_mw() for all QPs using that MW,
including those without a client. This also covers link-setup failures.

How to reproduce:

  1. Load ntb_transport and ntb_netdev on both sides and establish the
     transport/QP links once. Stop traffic, but leave ntb_netdev loaded
     on VHOST so its QPs remain allocated throughout the test.

  2. On HOST, unload ntb_netdev and ntb_transport, leaving ntb_hw_epf
     bound:

     modprobe -r ntb_netdev ntb_transport

     Transport removal sends COMMAND_LINK_DOWN to VHOST. Wait for
     ntb_transport_link_cleanup_work() to return on VHOST, using a
     function-graph trace. The "Link Cleanup" message is printed before
     MW release and is not sufficient to establish completion. Do not
     bring the link back up before the next step.

  3-(A). UAF via repeated link-down notification

     Use ntb_tool on HOST to send another link-down request:

     HOST# modprobe ntb_tool
     HOST# echo N > "/sys/kernel/debug/ntb_tool/$ntb_host_dev/link"

     ==================================================================
     BUG: KASAN: vmalloc-out-of-bounds in ntb_qp_link_down_reset+0x2c0..
     ...
     Call trace:
      ...
      __asan_report_store4_noabort+0x1c/0x28
      ntb_qp_link_down_reset+0x2c0/0x2e0 [ntb_transport]
      ntb_qp_link_cleanup+0xc4/0x148 [ntb_transport]
      ntb_transport_link_cleanup+0x314/0x350 [ntb_transport]
      ntb_transport_link_cleanup_work+0x2c/0x50 [ntb_transport]
      process_one_work+0x5b8/0x12f0
      ...

  3-(B). UAF via transport removal after link-down

     VHOST# echo "$ntb_vhost_dev" > \
         /sys/bus/ntb/drivers/ntb_transport/unbind

     ==================================================================
     BUG: KASAN: vmalloc-out-of-bounds in ntb_qp_link_down_reset+0x2c0..
     ...
     Call trace:
      ...
      __asan_report_store4_noabort+0x1c/0x28
      ntb_qp_link_down_reset+0x2c0/0x2e0 [ntb_transport]
      ntb_qp_link_cleanup+0xc4/0x148 [ntb_transport]
      ntb_transport_link_cleanup+0x314/0x350 [ntb_transport]
      ntb_transport_free+0x68/0x588 [ntb_transport]
      ntb_remove+0x5c/0xa0 [ntb]

Verified that neither test triggers a KASAN report with this patch.

Fixes: cc79bd2738c2 ("ntb: Clean up tx tail index on link down")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/ntb/ntb_transport.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index b949f36a4f2d..096be87e5ede 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -781,10 +781,17 @@ static void ntb_free_mw(struct ntb_transport_ctx *nt, int num_mw)
 {
 	struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
 	struct device *dma_dev = ntb_get_dma_dev(nt->ndev);
+	unsigned int i;
 
 	if (!mw->virt_addr)
 		return;
 
+	/* Drop references from every QP using this MW. */
+	for (i = num_mw; i < nt->qp_count; i += nt->mw_count) {
+		nt->qp_vec[i].rx_buff = NULL;
+		WRITE_ONCE(nt->qp_vec[i].remote_rx_info, NULL);
+	}
+
 	ntb_mw_clear_trans(nt->ndev, PIDX, num_mw);
 	dma_free_attrs(dma_dev, mw->alloc_size, mw->alloc_addr,
 		       mw->original_dma_addr, DMA_ATTR_FORCE_CONTIGUOUS);
-- 
2.51.0

[PATCH v2 13/14] NTB: ntb_transport: Abort link setup on QP MW allocation failure

[Koichiro Den]

ntb_transport_setup_qp_mw() can fail while growing a QP's RX entry pool,
but the link worker ignores that error. The worker can consequently
publish a QP whose memory-window state is only partly initialized.

Abort on the first QP setup error and release the MWs through the
existing error path instead of publishing the transport link.

Fixes: a754a8fcaf38 ("NTB: allocate number transport entries depending on size of ring size")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

Originally submitted as part of the direct TX/RX series v1:
https://lore.kernel.org/r/20260810165136.2292436-2-den@valinux.co.jp/

 drivers/ntb/ntb_transport.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 096be87e5ede..c1e3a077c45e 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1152,7 +1152,9 @@ static void ntb_transport_link_work(struct work_struct *work)
 	}
 
 	for (i = 0; i < nt->qp_count; i++) {
-		ntb_transport_setup_qp_mw(nt, i);
+		rc = ntb_transport_setup_qp_mw(nt, i);
+		if (rc)
+			goto out1;
 		ntb_transport_setup_qp_peer_msi(nt, i);
 	}
 
-- 
2.51.0

[PATCH v2 14/14] NTB: ntb_transport: Remove clients before freeing transport resources

[Koichiro Den]

Unbinding ntb_transport can call ntb_transport_free() while ntb_netdev
is still bound. The transport frees MWs and QP resources before
unregistering the clients, so the netdev's transmit path and timer can
access freed memory. Its remove callback also calls
ntb_transport_free_queue() on a QP whose resources have already been
released. This teardown order is unsafe and somewhat unintuitive.

The crash can be reproduced with an intensive TX load, during which you
unbind the NTB device. The following is a KASAN report from my
VHOST/HOST setup using vNTB.

  VHOST# sudo iperf3 -ub0 -c $HOST -l 100 -P 100 &
  VHOST# echo $VHOST_NTB_DEV > /sys/bus/ntb/drivers/ntb_transport/unbind

  ==================================================================
  BUG: KASAN: vmalloc-out-of-bounds in ntb_transport_tx_free_entry+0xf0
  ...
  Call trace:
   ...
   __asan_report_load4_noabort+0x1c/0x30
   ntb_transport_tx_free_entry+0xf0/0x130 [ntb_transport]
   ntb_netdev_tx_timer+0x78/0x260 [ntb_netdev]
   ...

Disable and drain transport link work first, then unregister the clients
so they stop using and release their QPs. After that, free any QPs left
over before running transport link cleanup. Disabling the work keeps
link events from restarting setup or cleanup during client removal.

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - New patch.

 drivers/ntb/ntb_transport.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index c1e3a077c45e..f24555b38d1b 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1487,9 +1487,11 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
 
 	debugfs_remove_recursive(nt->debugfs_node_dir);
 
-	ntb_transport_link_cleanup(nt);
-	cancel_work_sync(&nt->link_cleanup);
-	cancel_delayed_work_sync(&nt->link_work);
+	/* Stop transport work before clients release their QPs. */
+	disable_delayed_work_sync(&nt->link_work);
+	disable_work_sync(&nt->link_cleanup);
+
+	ntb_bus_remove(nt);
 
 	qp_bitmap_alloc = nt->qp_bitmap & ~nt->qp_bitmap_free;
 
@@ -1500,11 +1502,10 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
 			ntb_transport_free_queue(qp);
 	}
 
+	ntb_transport_link_cleanup(nt);
 	ntb_link_disable(ndev);
 	ntb_clear_ctx(ndev);
 
-	ntb_bus_remove(nt);
-
 	for (i = nt->mw_count; i--; ) {
 		ntb_free_mw(nt, i);
 		iounmap(nt->mw_vec[i].vbase);
-- 
2.51.0