[PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition

[PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition

[Alison Schofield]

BTT (Block Translation Table) makes persistent memory safe for block
I/O by guaranteeing atomic sector updates. It uses reserved lanes
for in-flight BTT operations, which must be used exclusively.

The btt-check unit test reports data mismatches during BTT I/O due
to a race in lane acquisition, leading to silent data corruption.

BTT lane acquisition uses per-CPU recursion tracking with
migrate_disable(). However, migrate_disable() does not prevent
preemption, so another task can run on the same CPU and share the
recursion state. That task can observe a non-zero recursion count,
bypass locking, and use the same lane at the same time.

Track lane ownership per task and only allow lockless recursion for
the owning task. Otherwise, serialize access with the lane spinlock.
Use spin_(un)lock_bh() so softirq re-entry on the same CPU cannot
bypass ownership checks or deadlock on the lane lock.

Found with the NDCTL unit test btt-check.sh

Fixes: 36c75ce3bd29 ("nd_btt: Make BTT lanes preemptible")
Assisted-by: Claude Sonnet 4.5
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

Changes in v2:
Use spin_(un)lock_bh() (Sashiko AI)
Update commit log per softirq re-enty and spinlock change

A new unit test to stress this is under review here:
https://lore.kernel.org/nvdimm/20260424233633.3762217-1-alison.schofield@intel.com/


 drivers/nvdimm/nd.h          |  1 +
 drivers/nvdimm/region_devs.c | 48 +++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index b199eea3260e..424c38ca4960 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -368,6 +368,7 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
 struct nd_percpu_lane {
 	int count;
 	spinlock_t lock;
+	struct task_struct *owner;
 };
 
 enum nd_label_flags {
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index e35c2e18518f..f1c6dcd95b5a 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -905,11 +905,10 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
  * @nd_region: region id and number of lanes possible
  *
  * A lane correlates to a BLK-data-window and/or a log slot in the BTT.
- * We optimize for the common case where there are 256 lanes, one
- * per-cpu.  For larger systems we need to lock to share lanes.  For now
- * this implementation assumes the cost of maintaining an allocator for
- * free lanes is on the order of the lock hold time, so it implements a
- * static lane = cpu % num_lanes mapping.
+ * Lanes are shared across CPUs using a static lane = cpu % num_lanes
+ * mapping, with a per-lane spinlock to serialize access when multiple
+ * tasks share a lane (including when preemption causes multiple tasks
+ * to run on the same CPU).
  *
  * In the case of a BTT instance on top of a BLK namespace a lane may be
  * acquired recursively.  We lock on the first instance.
@@ -920,35 +919,44 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
 unsigned int nd_region_acquire_lane(struct nd_region *nd_region)
 {
 	unsigned int cpu, lane;
+	struct nd_percpu_lane *ndl;
 
 	migrate_disable();
 	cpu = smp_processor_id();
-	if (nd_region->num_lanes < nr_cpu_ids) {
-		struct nd_percpu_lane *ndl_lock, *ndl_count;
-
+	if (nd_region->num_lanes < nr_cpu_ids)
 		lane = cpu % nd_region->num_lanes;
-		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
-		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
-		if (ndl_count->count++ == 0)
-			spin_lock(&ndl_lock->lock);
-	} else
+	else
 		lane = cpu;
 
+	/*
+	 * migrate_disable() keeps the lane stable, but does not prevent
+	 * preemption. Only the owning task may recurse without taking the
+	 * lock.
+	 */
+	ndl = per_cpu_ptr(nd_region->lane, lane);
+	if (READ_ONCE(ndl->owner) != current) {
+		spin_lock_bh(&ndl->lock);
+		WRITE_ONCE(ndl->owner, current);
+	}
+	ndl->count++;
+
 	return lane;
 }
 EXPORT_SYMBOL(nd_region_acquire_lane);
 
 void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
 {
-	if (nd_region->num_lanes < nr_cpu_ids) {
-		unsigned int cpu = smp_processor_id();
-		struct nd_percpu_lane *ndl_lock, *ndl_count;
+	struct nd_percpu_lane *ndl = per_cpu_ptr(nd_region->lane, lane);
 
-		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
-		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
-		if (--ndl_count->count == 0)
-			spin_unlock(&ndl_lock->lock);
+	if (WARN_ON_ONCE(READ_ONCE(ndl->owner) != current))
+		goto out;
+
+	if (--ndl->count == 0) {
+		WRITE_ONCE(ndl->owner, NULL);
+		spin_unlock_bh(&ndl->lock);
 	}
+
+out:
 	migrate_enable();
 }
 EXPORT_SYMBOL(nd_region_release_lane);

base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.37.3

Re: [PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition

[Aboorva Devarajan]

On Wed, 2026-04-29 at 19:46 -0700, Alison Schofield wrote:

> BTT (Block Translation Table) makes persistent memory safe for block
> I/O by guaranteeing atomic sector updates. It uses reserved lanes
> for in-flight BTT operations, which must be used exclusively.
> 
> The btt-check unit test reports data mismatches during BTT I/O due
> to a race in lane acquisition, leading to silent data corruption.
> 
> BTT lane acquisition uses per-CPU recursion tracking with
> migrate_disable(). However, migrate_disable() does not prevent
> preemption, so another task can run on the same CPU and share the
> recursion state. That task can observe a non-zero recursion count,
> bypass locking, and use the same lane at the same time.
> 
> Track lane ownership per task and only allow lockless recursion for
> the owning task. Otherwise, serialize access with the lane spinlock.
> Use spin_(un)lock_bh() so softirq re-entry on the same CPU cannot
> bypass ownership checks or deadlock on the lane lock.
> 
> Found with the NDCTL unit test btt-check.sh
> 
> Fixes: 36c75ce3bd29 ("nd_btt: Make BTT lanes preemptible")
> Assisted-by: Claude Sonnet 4.5
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> Changes in v2:
> Use spin_(un)lock_bh() (Sashiko AI)
> Update commit log per softirq re-enty and spinlock change
> 
> A new unit test to stress this is under review here:
> https://lore.kernel.org/nvdimm/20260424233633.3762217-1-alison.schofield@intel.com/
> 
> 
>  drivers/nvdimm/nd.h          |  1 +
>  drivers/nvdimm/region_devs.c | 48 +++++++++++++++++++++---------------
>  2 files changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index b199eea3260e..424c38ca4960 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -368,6 +368,7 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
>  struct nd_percpu_lane {
>  	int count;
>  	spinlock_t lock;
> +	struct task_struct *owner;
>  };
>  
>  enum nd_label_flags {
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index e35c2e18518f..f1c6dcd95b5a 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -905,11 +905,10 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
>   * @nd_region: region id and number of lanes possible
>   *
>   * A lane correlates to a BLK-data-window and/or a log slot in the BTT.
> - * We optimize for the common case where there are 256 lanes, one
> - * per-cpu.  For larger systems we need to lock to share lanes.  For now
> - * this implementation assumes the cost of maintaining an allocator for
> - * free lanes is on the order of the lock hold time, so it implements a
> - * static lane = cpu % num_lanes mapping.
> + * Lanes are shared across CPUs using a static lane = cpu % num_lanes
> + * mapping, with a per-lane spinlock to serialize access when multiple
> + * tasks share a lane (including when preemption causes multiple tasks
> + * to run on the same CPU).
>   *
>   * In the case of a BTT instance on top of a BLK namespace a lane may be
>   * acquired recursively.  We lock on the first instance.
> @@ -920,35 +919,44 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
>  unsigned int nd_region_acquire_lane(struct nd_region *nd_region)
>  {
>  	unsigned int cpu, lane;
> +	struct nd_percpu_lane *ndl;
>  
>  	migrate_disable();
>  	cpu = smp_processor_id();
> -	if (nd_region->num_lanes < nr_cpu_ids) {
> -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> -
> +	if (nd_region->num_lanes < nr_cpu_ids)
>  		lane = cpu % nd_region->num_lanes;
> -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> -		if (ndl_count->count++ == 0)
> -			spin_lock(&ndl_lock->lock);
> -	} else
> +	else
>  		lane = cpu;
>  
> +	/*
> +	 * migrate_disable() keeps the lane stable, but does not prevent
> +	 * preemption. Only the owning task may recurse without taking the
> +	 * lock.
> +	 */
> +	ndl = per_cpu_ptr(nd_region->lane, lane);
> +	if (READ_ONCE(ndl->owner) != current) {
> +		spin_lock_bh(&ndl->lock);
> +		WRITE_ONCE(ndl->owner, current);
> +	}
> +	ndl->count++;
> +
>  	return lane;
>  }
>  EXPORT_SYMBOL(nd_region_acquire_lane);
>  
>  void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
>  {
> -	if (nd_region->num_lanes < nr_cpu_ids) {
> -		unsigned int cpu = smp_processor_id();
> -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> +	struct nd_percpu_lane *ndl = per_cpu_ptr(nd_region->lane, lane);
>  
> -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> -		if (--ndl_count->count == 0)
> -			spin_unlock(&ndl_lock->lock);
> +	if (WARN_ON_ONCE(READ_ONCE(ndl->owner) != current))
> +		goto out;
> +
> +	if (--ndl->count == 0) {
> +		WRITE_ONCE(ndl->owner, NULL);
> +		spin_unlock_bh(&ndl->lock);
>  	}
> +
> +out:
>  	migrate_enable();
>  }
>  EXPORT_SYMBOL(nd_region_release_lane);
> 
> base-commit: 028ef9c96e96197026887c0f092424679298aae8


Hi Alison,

Thanks for the fix.

I noticed a similar race in BTT and wrote a small selftest to
exercise concurrent lane usage [1].

Without your patch on the almost latest upstream kernel v7.0.0, I see silent 
data corruption on this workload, most reliably with PREEMPT_DYNAMIC set to
"lazy" or "full". With your patch applied the test passes across all four preempt modes.

  
  # Without the patch
  echo none      -> pass
  echo voluntary -> pass
  echo lazy      -> "not ok 1 BTT lane contention: 6 process(es) saw corruption"
  echo full      -> "not ok 1 BTT lane contention: 8 process(es) saw corruption"
  

  
  # With the patch
  echo none / voluntary / lazy / full -> all pass

(Full output below)

So,

Tested-by: Aboorva Devarajan <aboorvad@linux.ibm.com>


------------------
Without Patch:
------------------

[nvdimm]# echo none > /sys/kernel/debug/sched/preempt
[nvdimm]# ./run_btt_lane_contention.sh
Creating sector-mode namespace on region1...
Namespace: namespace1.4
Block device: /dev/pmem1.4s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.4s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.4s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
ok 1 BTT lane contention: all data verified
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.4...
[nvdimm]# echo voluntary > /sys/kernel/debug/sched/preempt
[nvdimm]# cat /sys/kernel/debug/sched/preempt 
none (voluntary) full lazy 
[nvdimm]# ./run_btt_lane_contention.sh
Creating sector-mode namespace on region1...
Namespace: namespace1.3
Block device: /dev/pmem1.3s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.3s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.3s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
ok 1 BTT lane contention: all data verified
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.3...
[nvdimm]# echo lazy > /sys/kernel/debug/sched/preempt
[nvdimm]# cat /sys/kernel/debug/sched/preempt 
none voluntary full (lazy) 
[nvdimm]# ./run_btt_lane_contention.sh
Creating sector-mode namespace on region1...
Namespace: namespace1.4
Block device: /dev/pmem1.4s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.4s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.4s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
# [proc 10] MISCOMPARE iter=4 block=80 off=0x47180000
# [proc 10]   byte 204800: exp 0x4a got 0x42 (4096/262144 bad)
# [proc 10]   from proc 2 (shared CPU 2)
# [proc 12] MISCOMPARE iter=11 block=279 off=0x582c0000
# [proc 12]   byte 102400: exp 0x4c got 0x44 (4096/262144 bad)
# [proc 12]   from proc 4 (shared CPU 4)
# [proc 13] MISCOMPARE iter=19 block=143 off=0x5d080000
# [proc 14] MISCOMPARE iter=19 block=28 off=0x62380000
# [proc 14]   byte 28672: exp 0x4e got 0x46 (4096/262144 bad)
# [proc 14]   from proc 6 (shared CPU 6)
# [proc 13]   byte 249856: exp 0x4d got 0x45 (4096/262144 bad)
# [proc 13]   from proc 5 (shared CPU 5)
# [proc 1] MISCOMPARE iter=57 block=55 off=0x7d80000
# [proc 1]   byte 135168: exp 0x41 got 0x49 (4096/262144 bad)
# [proc 1]   from proc 9 (shared CPU 1)
# [proc 8] MISCOMPARE iter=71 block=392 off=0x3e000000
# [proc 8]   byte 208896: exp 0x48 got 0x40 (4096/262144 bad)
# [proc 8]   from proc 0 (shared CPU 0)
not ok 1 BTT lane contention: 6 process(es) saw corruption
# Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.4...
[nvdimm]# echo full > /sys/kernel/debug/sched/preempt
[nvdimm]# cat /sys/kernel/debug/sched/preempt 
none voluntary (full) lazy 
[nvdimm]# ./run_btt_lane_contention.sh
Creating sector-mode namespace on region1...
Namespace: namespace1.3
Block device: /dev/pmem1.3s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.3s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.3s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
# [proc 1] MISCOMPARE iter=0 block=3 off=0x7080000
# [proc 4] MISCOMPARE iter=0 block=3 off=0x1bfc0000
# [proc 4]   byte 114688: exp 0x44 got 0x4c (4096/262144 bad)
# [proc 7] MISCOMPARE iter=0 block=2 off=0x30ec0000
# [proc 7]   byte 212992: exp 0x47 got 0x4f (4096/262144 bad)
# [proc 1]   byte 159744: exp 0x41 got 0x49 (4096/262144 bad)
# [proc 1]   from proc 9 (shared CPU 1)
# [proc 4]   from proc 12 (shared CPU 4)
# [proc 7]   from proc 15 (shared CPU 7)
# [proc 10] MISCOMPARE iter=0 block=2 off=0x45e00000
# [proc 10]   byte 139264: exp 0x4a got 0x42 (1024/262144 bad)
# [proc 10]   from proc 2 (shared CPU 2)
# [proc 3] MISCOMPARE iter=0 block=9 off=0x15180000
# [proc 3]   byte 217088: exp 0x43 got 0x4b (4096/262144 bad)
# [proc 3]   from proc 11 (shared CPU 3)
# [proc 0] MISCOMPARE iter=0 block=9 off=0x240000
# [proc 0]   byte 159744: exp 0x40 got 0x48 (4096/262144 bad)
# [proc 0]   from proc 8 (shared CPU 0)
# [proc 5] MISCOMPARE iter=0 block=9 off=0x23100000
# [proc 5]   byte 122880: exp 0x45 got 0x4d (4096/262144 bad)
# [proc 5]   from proc 13 (shared CPU 5)
# [proc 6] MISCOMPARE iter=0 block=14 off=0x2a200000
# [proc 6]   byte 57344: exp 0x46 got 0x4e (4096/262144 bad)
# [proc 6]   from proc 14 (shared CPU 6)
not ok 1 BTT lane contention: 8 process(es) saw corruption
# Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.3...
[nvdimm]# 


------------------
With Patch:
------------------

[nvdimm]# cat /sys/kernel/debug/sched/preempt 
(none) voluntary full lazy 
[nvdimm]# ./run_btt_lane_contention.sh 
Creating sector-mode namespace on region1...
Namespace: namespace1.3
Block device: /dev/pmem1.3s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.3s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.3s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
ok 1 BTT lane contention: all data verified
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.3...
[nvdimm]# echo voluntary > /sys/kernel/debug/sched/preempt
[nvdimm]# cat /sys/kernel/debug/sched/preempt 
none (voluntary) full lazy 
[nvdimm]# ./run_btt_lane_contention.sh 
Creating sector-mode namespace on region1...
Namespace: namespace1.4
Block device: /dev/pmem1.4s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.4s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.4s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
ok 1 BTT lane contention: all data verified
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.4...
[nvdimm]# echo lazy > /sys/kernel/debug/sched/preempt
[nvdimm]# cat /sys/kernel/debug/sched/preempt 
none voluntary full (lazy) 
[nvdimm]# ./run_btt_lane_contention.sh 
Creating sector-mode namespace on region1...
Namespace: namespace1.3
Block device: /dev/pmem1.3s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.3s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.3s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
ok 1 BTT lane contention: all data verified
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.3...
[nvdimm]# echo full > /sys/kernel/debug/sched/preempt
[nvdimm]# cat /sys/kernel/debug/sched/preempt 
none voluntary (full) lazy 
[nvdimm]# ./run_btt_lane_contention.sh 
Creating sector-mode namespace on region1...
Namespace: namespace1.4
Block device: /dev/pmem1.4s
Running: /home/abd/linux/tools/testing/selftests/nvdimm/btt_lane_contention /dev/pmem1.4s 16 100
---
TAP version 13
1..1
# device: /dev/pmem1.4s (1789 MB)
# processes: 16 (2 per CPU across 8 CPUs)
# iterations: 100 per process
# I/O size: 256 KB
# logical block size: 4096 bytes
ok 1 BTT lane contention: all data verified
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
Cleaning up namespace namespace1.4...

[1] https://github.com/AboorvaDevarajan/linux/blob/btt_preempt_test/v1/tools/testing/selftests/nvdimm/btt_lane_contention.c


Regards,
Aboorva

Re: [PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition

[Aboorva Devarajan]

On Wed, 2026-04-29 at 19:46 -0700, Alison Schofield wrote:
> BTT (Block Translation Table) makes persistent memory safe for block
> I/O by guaranteeing atomic sector updates. It uses reserved lanes
> for in-flight BTT operations, which must be used exclusively.
> 
> The btt-check unit test reports data mismatches during BTT I/O due
> to a race in lane acquisition, leading to silent data corruption.
> 
> BTT lane acquisition uses per-CPU recursion tracking with
> migrate_disable(). However, migrate_disable() does not prevent
> preemption, so another task can run on the same CPU and share the
> recursion state. That task can observe a non-zero recursion count,
> bypass locking, and use the same lane at the same time.
> 
> Track lane ownership per task and only allow lockless recursion for
> the owning task. Otherwise, serialize access with the lane spinlock.
> Use spin_(un)lock_bh() so softirq re-entry on the same CPU cannot
> bypass ownership checks or deadlock on the lane lock.
> 
> Found with the NDCTL unit test btt-check.sh
> 
> Fixes: 36c75ce3bd29 ("nd_btt: Make BTT lanes preemptible")
> Assisted-by: Claude Sonnet 4.5
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> Changes in v2:
> Use spin_(un)lock_bh() (Sashiko AI)
> Update commit log per softirq re-enty and spinlock change
> 
> A new unit test to stress this is under review here:
> https://lore.kernel.org/nvdimm/20260424233633.3762217-1-alison.schofield@intel.com/
> 
> 
>  drivers/nvdimm/nd.h          |  1 +
>  drivers/nvdimm/region_devs.c | 48 +++++++++++++++++++++---------------
>  2 files changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index b199eea3260e..424c38ca4960 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -368,6 +368,7 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
>  struct nd_percpu_lane {
>  	int count;
>  	spinlock_t lock;
> +	struct task_struct *owner;
>  };
>  
>  enum nd_label_flags {
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index e35c2e18518f..f1c6dcd95b5a 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -905,11 +905,10 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
>   * @nd_region: region id and number of lanes possible
>   *
>   * A lane correlates to a BLK-data-window and/or a log slot in the BTT.
> - * We optimize for the common case where there are 256 lanes, one
> - * per-cpu.  For larger systems we need to lock to share lanes.  For now
> - * this implementation assumes the cost of maintaining an allocator for
> - * free lanes is on the order of the lock hold time, so it implements a
> - * static lane = cpu % num_lanes mapping.
> + * Lanes are shared across CPUs using a static lane = cpu % num_lanes
> + * mapping, with a per-lane spinlock to serialize access when multiple
> + * tasks share a lane (including when preemption causes multiple tasks
> + * to run on the same CPU).
>   *
>   * In the case of a BTT instance on top of a BLK namespace a lane may be
>   * acquired recursively.  We lock on the first instance.
> @@ -920,35 +919,44 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
>  unsigned int nd_region_acquire_lane(struct nd_region *nd_region)
>  {
>  	unsigned int cpu, lane;
> +	struct nd_percpu_lane *ndl;
>  
>  	migrate_disable();
>  	cpu = smp_processor_id();
> -	if (nd_region->num_lanes < nr_cpu_ids) {
> -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> -
> +	if (nd_region->num_lanes < nr_cpu_ids)
>  		lane = cpu % nd_region->num_lanes;
> -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> -		if (ndl_count->count++ == 0)
> -			spin_lock(&ndl_lock->lock);
> -	} else
> +	else
>  		lane = cpu;
>  
> +	/*
> +	 * migrate_disable() keeps the lane stable, but does not prevent
> +	 * preemption. Only the owning task may recurse without taking the
> +	 * lock.
> +	 */
> +	ndl = per_cpu_ptr(nd_region->lane, lane);
> +	if (READ_ONCE(ndl->owner) != current) {
> +		spin_lock_bh(&ndl->lock);
> +		WRITE_ONCE(ndl->owner, current);
> +	}
> +	ndl->count++;
> +
>  	return lane;
>  }
>  EXPORT_SYMBOL(nd_region_acquire_lane);
>  
>  void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
>  {
> -	if (nd_region->num_lanes < nr_cpu_ids) {
> -		unsigned int cpu = smp_processor_id();
> -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> +	struct nd_percpu_lane *ndl = per_cpu_ptr(nd_region->lane, lane);
>  
> -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> -		if (--ndl_count->count == 0)
> -			spin_unlock(&ndl_lock->lock);
> +	if (WARN_ON_ONCE(READ_ONCE(ndl->owner) != current))
> +		goto out;
> +
> +	if (--ndl->count == 0) {
> +		WRITE_ONCE(ndl->owner, NULL);
> +		spin_unlock_bh(&ndl->lock);
>  	}
> +
> +out:
>  	migrate_enable();
>  }
>  EXPORT_SYMBOL(nd_region_release_lane);
> 
> base-commit: 028ef9c96e96197026887c0f092424679298aae8

Hi Alison,

Just a follow-up question.

I haven't reproduced this, just noticed it while reading the code.

After this patch, nd_region_acquire_lane() / nd_region_release_lane() always
hold a spinlock, IIUC, anything that sleeps/blocks in this critical section will
hit:

    BUG: scheduling while atomic: ...

BTT metadata writes go arena_write_bytes() -> nvdimm_write_bytes() ->
nsio_rw_bytes(), which always calls nvdimm_flush() on write. That can call
nd_region->flush():

  - virtio_pmem_flush() uses a wait_event(), so it can block on
    every flush.

  - papr_scm_pmem_flush() only msleep() when the flush hcall
    comes back busy; the fast path does not sleep, though this is rare case.

So BTT on virtio_pmem looks like it could trip the BUG on metadata
writes, papr_scm only if the busy path is taken? Pre-patch, the same behaviour
already existed on > 256-CPU boxes where the lane spinlock was taken.

Is this an actual concern, so are we essentially saying that no sleep /
blocking wait is allowed anywhere reachable from the lane critical section?

Please correct me if I'm missing something here.

Thanks,
Aboorva

Re: [PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition

[Alison Schofield]

On Fri, May 01, 2026 at 05:01:15PM +0530, Aboorva Devarajan wrote:
> On Wed, 2026-04-29 at 19:46 -0700, Alison Schofield wrote:
> > BTT (Block Translation Table) makes persistent memory safe for block
> > I/O by guaranteeing atomic sector updates. It uses reserved lanes
> > for in-flight BTT operations, which must be used exclusively.
> > 
> > The btt-check unit test reports data mismatches during BTT I/O due
> > to a race in lane acquisition, leading to silent data corruption.
> > 
> > BTT lane acquisition uses per-CPU recursion tracking with
> > migrate_disable(). However, migrate_disable() does not prevent
> > preemption, so another task can run on the same CPU and share the
> > recursion state. That task can observe a non-zero recursion count,
> > bypass locking, and use the same lane at the same time.
> > 
> > Track lane ownership per task and only allow lockless recursion for
> > the owning task. Otherwise, serialize access with the lane spinlock.
> > Use spin_(un)lock_bh() so softirq re-entry on the same CPU cannot
> > bypass ownership checks or deadlock on the lane lock.
> > 
> > Found with the NDCTL unit test btt-check.sh
> > 
> > Fixes: 36c75ce3bd29 ("nd_btt: Make BTT lanes preemptible")
> > Assisted-by: Claude Sonnet 4.5
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> > 
> > Changes in v2:
> > Use spin_(un)lock_bh() (Sashiko AI)
> > Update commit log per softirq re-enty and spinlock change
> > 
> > A new unit test to stress this is under review here:
> > https://lore.kernel.org/nvdimm/20260424233633.3762217-1-alison.schofield@intel.com/
> > 
> > 
> >  drivers/nvdimm/nd.h          |  1 +
> >  drivers/nvdimm/region_devs.c | 48 +++++++++++++++++++++---------------
> >  2 files changed, 29 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> > index b199eea3260e..424c38ca4960 100644
> > --- a/drivers/nvdimm/nd.h
> > +++ b/drivers/nvdimm/nd.h
> > @@ -368,6 +368,7 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
> >  struct nd_percpu_lane {
> >  	int count;
> >  	spinlock_t lock;
> > +	struct task_struct *owner;
> >  };
> >  
> >  enum nd_label_flags {
> > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> > index e35c2e18518f..f1c6dcd95b5a 100644
> > --- a/drivers/nvdimm/region_devs.c
> > +++ b/drivers/nvdimm/region_devs.c
> > @@ -905,11 +905,10 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
> >   * @nd_region: region id and number of lanes possible
> >   *
> >   * A lane correlates to a BLK-data-window and/or a log slot in the BTT.
> > - * We optimize for the common case where there are 256 lanes, one
> > - * per-cpu.  For larger systems we need to lock to share lanes.  For now
> > - * this implementation assumes the cost of maintaining an allocator for
> > - * free lanes is on the order of the lock hold time, so it implements a
> > - * static lane = cpu % num_lanes mapping.
> > + * Lanes are shared across CPUs using a static lane = cpu % num_lanes
> > + * mapping, with a per-lane spinlock to serialize access when multiple
> > + * tasks share a lane (including when preemption causes multiple tasks
> > + * to run on the same CPU).
> >   *
> >   * In the case of a BTT instance on top of a BLK namespace a lane may be
> >   * acquired recursively.  We lock on the first instance.
> > @@ -920,35 +919,44 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
> >  unsigned int nd_region_acquire_lane(struct nd_region *nd_region)
> >  {
> >  	unsigned int cpu, lane;
> > +	struct nd_percpu_lane *ndl;
> >  
> >  	migrate_disable();
> >  	cpu = smp_processor_id();
> > -	if (nd_region->num_lanes < nr_cpu_ids) {
> > -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> > -
> > +	if (nd_region->num_lanes < nr_cpu_ids)
> >  		lane = cpu % nd_region->num_lanes;
> > -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> > -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> > -		if (ndl_count->count++ == 0)
> > -			spin_lock(&ndl_lock->lock);
> > -	} else
> > +	else
> >  		lane = cpu;
> >  
> > +	/*
> > +	 * migrate_disable() keeps the lane stable, but does not prevent
> > +	 * preemption. Only the owning task may recurse without taking the
> > +	 * lock.
> > +	 */
> > +	ndl = per_cpu_ptr(nd_region->lane, lane);
> > +	if (READ_ONCE(ndl->owner) != current) {
> > +		spin_lock_bh(&ndl->lock);
> > +		WRITE_ONCE(ndl->owner, current);
> > +	}
> > +	ndl->count++;
> > +
> >  	return lane;
> >  }
> >  EXPORT_SYMBOL(nd_region_acquire_lane);
> >  
> >  void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
> >  {
> > -	if (nd_region->num_lanes < nr_cpu_ids) {
> > -		unsigned int cpu = smp_processor_id();
> > -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> > +	struct nd_percpu_lane *ndl = per_cpu_ptr(nd_region->lane, lane);
> >  
> > -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> > -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> > -		if (--ndl_count->count == 0)
> > -			spin_unlock(&ndl_lock->lock);
> > +	if (WARN_ON_ONCE(READ_ONCE(ndl->owner) != current))
> > +		goto out;
> > +
> > +	if (--ndl->count == 0) {
> > +		WRITE_ONCE(ndl->owner, NULL);
> > +		spin_unlock_bh(&ndl->lock);
> >  	}
> > +
> > +out:
> >  	migrate_enable();
> >  }
> >  EXPORT_SYMBOL(nd_region_release_lane);
> > 
> > base-commit: 028ef9c96e96197026887c0f092424679298aae8
> 
> Hi Alison,
> 
> Just a follow-up question.
> 
> I haven't reproduced this, just noticed it while reading the code.
> 
> After this patch, nd_region_acquire_lane() / nd_region_release_lane() always
> hold a spinlock, IIUC, anything that sleeps/blocks in this critical section will
> hit:
> 
>     BUG: scheduling while atomic: ...
> 
> BTT metadata writes go arena_write_bytes() -> nvdimm_write_bytes() ->
> nsio_rw_bytes(), which always calls nvdimm_flush() on write. That can call
> nd_region->flush():
> 
>   - virtio_pmem_flush() uses a wait_event(), so it can block on
>     every flush.
> 
>   - papr_scm_pmem_flush() only msleep() when the flush hcall
>     comes back busy; the fast path does not sleep, though this is rare case.
> 
> So BTT on virtio_pmem looks like it could trip the BUG on metadata
> writes, papr_scm only if the busy path is taken? Pre-patch, the same behaviour
> already existed on > 256-CPU boxes where the lane spinlock was taken.
> 
> Is this an actual concern, so are we essentially saying that no sleep /
> blocking wait is allowed anywhere reachable from the lane critical section?
> 
> Please correct me if I'm missing something here.

Thanks for the review. You found a real issue.

The BTT lane lock is held across BTT write paths that can reach
nvdimm_flush(), and provider flush callbacks (e.g. virtio_pmem and
papr_scm) can sleep. So the current design incorrectly assumes that
the lane critical section is fully atomic.

As you pointed out, this predates this patch. The shared-lane path
has held a spinlock across this same call chain since the original
BTT merge. This patch probably widens the exposure by taking the lock
unconditionally.

I'm reworking this as a small series. The first patch converts the
per-lane lock to a mutex so the lane critical section can safely
sleep.

I appreciate your testing and will probaly need to rely on it more
in the next version.

Thanks,
Alison

> 
> Thanks,
> Aboorva

Re: [PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition

[Aboorva Devarajan]

On Fri, 2026-05-01 at 22:18 -0700, Alison Schofield wrote:
> On Fri, May 01, 2026 at 05:01:15PM +0530, Aboorva Devarajan wrote:
> > On Wed, 2026-04-29 at 19:46 -0700, Alison Schofield wrote:
> > > BTT (Block Translation Table) makes persistent memory safe for block
> > > I/O by guaranteeing atomic sector updates. It uses reserved lanes
> > > for in-flight BTT operations, which must be used exclusively.
> > > 
> > > The btt-check unit test reports data mismatches during BTT I/O due
> > > to a race in lane acquisition, leading to silent data corruption.
> > > 
> > > BTT lane acquisition uses per-CPU recursion tracking with
> > > migrate_disable(). However, migrate_disable() does not prevent
> > > preemption, so another task can run on the same CPU and share the
> > > recursion state. That task can observe a non-zero recursion count,
> > > bypass locking, and use the same lane at the same time.
> > > 
> > > Track lane ownership per task and only allow lockless recursion for
> > > the owning task. Otherwise, serialize access with the lane spinlock.
> > > Use spin_(un)lock_bh() so softirq re-entry on the same CPU cannot
> > > bypass ownership checks or deadlock on the lane lock.
> > > 
> > > Found with the NDCTL unit test btt-check.sh
> > > 
> > > Fixes: 36c75ce3bd29 ("nd_btt: Make BTT lanes preemptible")
> > > Assisted-by: Claude Sonnet 4.5
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > ---
> > > 
> > > Changes in v2:
> > > Use spin_(un)lock_bh() (Sashiko AI)
> > > Update commit log per softirq re-enty and spinlock change
> > > 
> > > A new unit test to stress this is under review here:
> > > https://lore.kernel.org/nvdimm/20260424233633.3762217-1-alison.schofield@intel.com/
> > > 
> > > 
> > >  drivers/nvdimm/nd.h          |  1 +
> > >  drivers/nvdimm/region_devs.c | 48 +++++++++++++++++++++---------------
> > >  2 files changed, 29 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> > > index b199eea3260e..424c38ca4960 100644
> > > --- a/drivers/nvdimm/nd.h
> > > +++ b/drivers/nvdimm/nd.h
> > > @@ -368,6 +368,7 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
> > >  struct nd_percpu_lane {
> > >  	int count;
> > >  	spinlock_t lock;
> > > +	struct task_struct *owner;
> > >  };
> > >  
> > >  enum nd_label_flags {
> > > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> > > index e35c2e18518f..f1c6dcd95b5a 100644
> > > --- a/drivers/nvdimm/region_devs.c
> > > +++ b/drivers/nvdimm/region_devs.c
> > > @@ -905,11 +905,10 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
> > >   * @nd_region: region id and number of lanes possible
> > >   *
> > >   * A lane correlates to a BLK-data-window and/or a log slot in the BTT.
> > > - * We optimize for the common case where there are 256 lanes, one
> > > - * per-cpu.  For larger systems we need to lock to share lanes.  For now
> > > - * this implementation assumes the cost of maintaining an allocator for
> > > - * free lanes is on the order of the lock hold time, so it implements a
> > > - * static lane = cpu % num_lanes mapping.
> > > + * Lanes are shared across CPUs using a static lane = cpu % num_lanes
> > > + * mapping, with a per-lane spinlock to serialize access when multiple
> > > + * tasks share a lane (including when preemption causes multiple tasks
> > > + * to run on the same CPU).
> > >   *
> > >   * In the case of a BTT instance on top of a BLK namespace a lane may be
> > >   * acquired recursively.  We lock on the first instance.
> > > @@ -920,35 +919,44 @@ void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
> > >  unsigned int nd_region_acquire_lane(struct nd_region *nd_region)
> > >  {
> > >  	unsigned int cpu, lane;
> > > +	struct nd_percpu_lane *ndl;
> > >  
> > >  	migrate_disable();
> > >  	cpu = smp_processor_id();
> > > -	if (nd_region->num_lanes < nr_cpu_ids) {
> > > -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> > > -
> > > +	if (nd_region->num_lanes < nr_cpu_ids)
> > >  		lane = cpu % nd_region->num_lanes;
> > > -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> > > -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> > > -		if (ndl_count->count++ == 0)
> > > -			spin_lock(&ndl_lock->lock);
> > > -	} else
> > > +	else
> > >  		lane = cpu;
> > >  
> > > +	/*
> > > +	 * migrate_disable() keeps the lane stable, but does not prevent
> > > +	 * preemption. Only the owning task may recurse without taking the
> > > +	 * lock.
> > > +	 */
> > > +	ndl = per_cpu_ptr(nd_region->lane, lane);
> > > +	if (READ_ONCE(ndl->owner) != current) {
> > > +		spin_lock_bh(&ndl->lock);
> > > +		WRITE_ONCE(ndl->owner, current);
> > > +	}
> > > +	ndl->count++;
> > > +
> > >  	return lane;
> > >  }
> > >  EXPORT_SYMBOL(nd_region_acquire_lane);
> > >  
> > >  void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
> > >  {
> > > -	if (nd_region->num_lanes < nr_cpu_ids) {
> > > -		unsigned int cpu = smp_processor_id();
> > > -		struct nd_percpu_lane *ndl_lock, *ndl_count;
> > > +	struct nd_percpu_lane *ndl = per_cpu_ptr(nd_region->lane, lane);
> > >  
> > > -		ndl_count = per_cpu_ptr(nd_region->lane, cpu);
> > > -		ndl_lock = per_cpu_ptr(nd_region->lane, lane);
> > > -		if (--ndl_count->count == 0)
> > > -			spin_unlock(&ndl_lock->lock);
> > > +	if (WARN_ON_ONCE(READ_ONCE(ndl->owner) != current))
> > > +		goto out;
> > > +
> > > +	if (--ndl->count == 0) {
> > > +		WRITE_ONCE(ndl->owner, NULL);
> > > +		spin_unlock_bh(&ndl->lock);
> > >  	}
> > > +
> > > +out:
> > >  	migrate_enable();
> > >  }
> > >  EXPORT_SYMBOL(nd_region_release_lane);
> > > 
> > > base-commit: 028ef9c96e96197026887c0f092424679298aae8
> > 
> > Hi Alison,
> > 
> > Just a follow-up question.
> > 
> > I haven't reproduced this, just noticed it while reading the code.
> > 
> > After this patch, nd_region_acquire_lane() / nd_region_release_lane() always
> > hold a spinlock, IIUC, anything that sleeps/blocks in this critical section will
> > hit:
> > 
> >     BUG: scheduling while atomic: ...
> > 
> > BTT metadata writes go arena_write_bytes() -> nvdimm_write_bytes() ->
> > nsio_rw_bytes(), which always calls nvdimm_flush() on write. That can call
> > nd_region->flush():
> > 
> >   - virtio_pmem_flush() uses a wait_event(), so it can block on
> >     every flush.
> > 
> >   - papr_scm_pmem_flush() only msleep() when the flush hcall
> >     comes back busy; the fast path does not sleep, though this is rare case.
> > 
> > So BTT on virtio_pmem looks like it could trip the BUG on metadata
> > writes, papr_scm only if the busy path is taken? Pre-patch, the same behaviour
> > already existed on > 256-CPU boxes where the lane spinlock was taken.
> > 
> > Is this an actual concern, so are we essentially saying that no sleep /
> > blocking wait is allowed anywhere reachable from the lane critical section?
> > 
> > Please correct me if I'm missing something here.
> 
> Thanks for the review. You found a real issue.
> 
> The BTT lane lock is held across BTT write paths that can reach
> nvdimm_flush(), and provider flush callbacks (e.g. virtio_pmem and
> papr_scm) can sleep. So the current design incorrectly assumes that
> the lane critical section is fully atomic.
> 
> As you pointed out, this predates this patch. The shared-lane path
> has held a spinlock across this same call chain since the original
> BTT merge. This patch probably widens the exposure by taking the lock
> unconditionally.
> 
> I'm reworking this as a small series. The first patch converts the
> per-lane lock to a mutex so the lane critical section can safely
> sleep.

sure Alison, Thanks.

> 
> I appreciate your testing and will probaly need to rely on it more
> in the next version.
> 
> Thanks,
> Alison
> 
> > 
> > Thanks,
> > Aboorva