From: Aboorva Devarajan <aboorvad@linux.ibm.com>
To: Alison Schofield <alison.schofield@intel.com>
Cc: nvdimm@lists.linux.dev, Dan Williams <djbw@kernel.org>,
Vishal Verma <vishal.l.verma@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
Ira Weiny <ira.weiny@intel.com>,
aboorvad@linux.ibm.com
Subject: Re: [PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition
Date: Fri, 01 May 2026 16:27:16 +0530 [thread overview]
Message-ID: <7081312ccb45b067cd414a92c923f69cf1285097.camel@linux.ibm.com> (raw)
In-Reply-To: <20260430024652.3920875-1-alison.schofield@intel.com>
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
next prev parent reply other threads:[~2026-05-01 10:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-30 2:46 [PATCH v2] nvdimm/btt: Handle preemption in BTT lane acquisition Alison Schofield
2026-05-01 10:57 ` Aboorva Devarajan [this message]
2026-05-01 11:31 ` Aboorva Devarajan
2026-05-02 5:18 ` Alison Schofield
2026-05-03 16:17 ` Aboorva Devarajan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7081312ccb45b067cd414a92c923f69cf1285097.camel@linux.ibm.com \
--to=aboorvad@linux.ibm.com \
--cc=alison.schofield@intel.com \
--cc=dave.jiang@intel.com \
--cc=djbw@kernel.org \
--cc=ira.weiny@intel.com \
--cc=nvdimm@lists.linux.dev \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox