[PATCH v3 1/2] loop: use READ_ONCE() to read lo->lo_state without locking

[PATCH v3 1/2] loop: use READ_ONCE() to read lo->lo_state without locking

[Yongpeng Yang]

From: Yongpeng Yang <yangyongpeng@xiaomi.com>

When lo->lo_mutex is not held, direct access may read stale data. This
patch uses READ_ONCE() to read lo->lo_state and data_race() to silence
code checkers, and changes all assignments to use WRITE_ONCE().

Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
---
v3:
- Use WRITE_ONCE() instead of assignments to update lo_state.
v2:
- Use READ_ONCE() instead of converting lo_state to atomic_t type.
---
 drivers/block/loop.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 272bc608e528..b196387a85cf 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1082,7 +1082,7 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 	/* Order wrt reading lo_state in loop_validate_file(). */
 	wmb();
 
-	lo->lo_state = Lo_bound;
+	WRITE_ONCE(lo->lo_state, Lo_bound);
 	if (part_shift)
 		lo->lo_flags |= LO_FLAGS_PARTSCAN;
 	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
@@ -1179,7 +1179,7 @@ static void __loop_clr_fd(struct loop_device *lo)
 	if (!part_shift)
 		set_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
 	mutex_lock(&lo->lo_mutex);
-	lo->lo_state = Lo_unbound;
+	WRITE_ONCE(lo->lo_state, Lo_unbound);
 	mutex_unlock(&lo->lo_mutex);
 
 	/*
@@ -1218,7 +1218,7 @@ static int loop_clr_fd(struct loop_device *lo)
 
 	lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
 	if (disk_openers(lo->lo_disk) == 1)
-		lo->lo_state = Lo_rundown;
+		WRITE_ONCE(lo->lo_state, Lo_rundown);
 	loop_global_unlock(lo, true);
 
 	return 0;
@@ -1743,7 +1743,7 @@ static void lo_release(struct gendisk *disk)
 
 	mutex_lock(&lo->lo_mutex);
 	if (lo->lo_state == Lo_bound && (lo->lo_flags & LO_FLAGS_AUTOCLEAR))
-		lo->lo_state = Lo_rundown;
+		WRITE_ONCE(lo->lo_state, Lo_rundown);
 
 	need_clear = (lo->lo_state == Lo_rundown);
 	mutex_unlock(&lo->lo_mutex);
@@ -1858,7 +1858,7 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 
 	blk_mq_start_request(rq);
 
-	if (lo->lo_state != Lo_bound)
+	if (data_race(READ_ONCE(lo->lo_state)) != Lo_bound)
 		return BLK_STS_IOERR;
 
 	switch (req_op(rq)) {
@@ -2016,7 +2016,7 @@ static int loop_add(int i)
 	lo->worker_tree = RB_ROOT;
 	INIT_LIST_HEAD(&lo->idle_worker_list);
 	timer_setup(&lo->timer, loop_free_idle_workers_timer, TIMER_DEFERRABLE);
-	lo->lo_state = Lo_unbound;
+	WRITE_ONCE(lo->lo_state, Lo_unbound);
 
 	err = mutex_lock_killable(&loop_ctl_mutex);
 	if (err)
@@ -2174,7 +2174,7 @@ static int loop_control_remove(int idx)
 		goto mark_visible;
 	}
 	/* Mark this loop device as no more bound, but not quite unbound yet */
-	lo->lo_state = Lo_deleting;
+	WRITE_ONCE(lo->lo_state, Lo_deleting);
 	mutex_unlock(&lo->lo_mutex);
 
 	loop_remove(lo);
@@ -2198,7 +2198,7 @@ static int loop_control_get_free(int idx)
 		return ret;
 	idr_for_each_entry(&loop_index_idr, lo, id) {
 		/* Hitting a race results in creating a new loop device which is harmless. */
-		if (lo->idr_visible && data_race(lo->lo_state) == Lo_unbound)
+		if (lo->idr_visible && data_race(READ_ONCE(lo->lo_state)) == Lo_unbound)
 			goto found;
 	}
 	mutex_unlock(&loop_ctl_mutex);
-- 
2.43.0

[PATCH v3 2/2] zloop: use READ_ONCE() to read lo->lo_state in queue_rq path

[Yongpeng Yang]

From: Yongpeng Yang <yangyongpeng@xiaomi.com>

In the queue_rq path, zlo->state is accessed without locking, and direct
access may read stale data. This patch uses READ_ONCE() to read
zlo->state and data_race() to silence code checkers, and changes all
assignments to use WRITE_ONCE().

Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
---
v3:
- Use WRITE_ONCE() instead of assignments to update state.
v2:
- Use READ_ONCE() instead of converting state to atomic_t type.
---
 drivers/block/zloop.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/zloop.c b/drivers/block/zloop.c
index 77bd6081b244..8e334f5025fc 100644
--- a/drivers/block/zloop.c
+++ b/drivers/block/zloop.c
@@ -697,7 +697,7 @@ static blk_status_t zloop_queue_rq(struct blk_mq_hw_ctx *hctx,
 	struct zloop_cmd *cmd = blk_mq_rq_to_pdu(rq);
 	struct zloop_device *zlo = rq->q->queuedata;
 
-	if (zlo->state == Zlo_deleting)
+	if (data_race(READ_ONCE(zlo->state)) == Zlo_deleting)
 		return BLK_STS_IOERR;
 
 	/*
@@ -1002,7 +1002,7 @@ static int zloop_ctl_add(struct zloop_options *opts)
 		ret = -ENOMEM;
 		goto out;
 	}
-	zlo->state = Zlo_creating;
+	WRITE_ONCE(zlo->state, Zlo_creating);
 
 	ret = mutex_lock_killable(&zloop_ctl_mutex);
 	if (ret)
@@ -1113,7 +1113,7 @@ static int zloop_ctl_add(struct zloop_options *opts)
 	}
 
 	mutex_lock(&zloop_ctl_mutex);
-	zlo->state = Zlo_live;
+	WRITE_ONCE(zlo->state, Zlo_live);
 	mutex_unlock(&zloop_ctl_mutex);
 
 	pr_info("zloop: device %d, %u zones of %llu MiB, %u B block size\n",
@@ -1177,7 +1177,7 @@ static int zloop_ctl_remove(struct zloop_options *opts)
 		ret = -EINVAL;
 	} else {
 		idr_remove(&zloop_index_idr, zlo->id);
-		zlo->state = Zlo_deleting;
+		WRITE_ONCE(zlo->state, Zlo_deleting);
 	}
 
 	mutex_unlock(&zloop_ctl_mutex);
-- 
2.43.0

Re: [PATCH v3 2/2] zloop: use READ_ONCE() to read lo->lo_state in queue_rq path

[Damien Le Moal]

On 12/15/25 18:45, Yongpeng Yang wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> In the queue_rq path, zlo->state is accessed without locking, and direct
> access may read stale data. This patch uses READ_ONCE() to read
> zlo->state and data_race() to silence code checkers, and changes all
> assignments to use WRITE_ONCE().
> 
> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>

Looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 2/2] zloop: use READ_ONCE() to read lo->lo_state in queue_rq path

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v3 1/2] loop: use READ_ONCE() to read lo->lo_state without locking

[Damien Le Moal]

On 12/15/25 18:45, Yongpeng Yang wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> When lo->lo_mutex is not held, direct access may read stale data. This
> patch uses READ_ONCE() to read lo->lo_state and data_race() to silence
> code checkers, and changes all assignments to use WRITE_ONCE().
> 
> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>

Looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 1/2] loop: use READ_ONCE() to read lo->lo_state without locking

[Christoph Hellwig]

On Mon, Dec 15, 2025 at 05:45:22PM +0800, Yongpeng Yang wrote:
>  	idr_for_each_entry(&loop_index_idr, lo, id) {
>  		/* Hitting a race results in creating a new loop device which is harmless. */
> -		if (lo->idr_visible && data_race(lo->lo_state) == Lo_unbound)
> +		if (lo->idr_visible && data_race(READ_ONCE(lo->lo_state)) == Lo_unbound)

Please avoid the overly long line here (and maybe fix the one above).

Re: [PATCH v3 1/2] loop: use READ_ONCE() to read lo->lo_state without locking

[Yongpeng Yang]

On 12/15/25 10:38 PM, Christoph Hellwig wrote:
> On Mon, Dec 15, 2025 at 05:45:22PM +0800, Yongpeng Yang wrote:
>>   	idr_for_each_entry(&loop_index_idr, lo, id) {
>>   		/* Hitting a race results in creating a new loop device which is harmless. */
>> -		if (lo->idr_visible && data_race(lo->lo_state) == Lo_unbound)
>> +		if (lo->idr_visible && data_race(READ_ONCE(lo->lo_state)) == Lo_unbound)
> 
> Please avoid the overly long line here (and maybe fix the one above).
> 

OK, I'll fix this in v4.

Thanks
Yongpeng,