[PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn

[Zizhi Wo]

From: Zizhi Wo <wozizhi@xxxxxxxxxx>

The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
after apply_fn() returns. For attributes with an apply_fn (submit_queues,
poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.

configfs serializes writes via a per-open-file mutex (buffer->mutex), so
two threads writing to the same attribute through separate open file
descriptions run the store callback concurrently. The macro's write is
redundant and lockless, so a concurrent store's losing thread can overwrite
the winner's value after apply_fn set it, making dev->submit_queues
mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
falls back to a single queue.

Restructure the macro so that apply_fn attributes return directly after
apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
only changeable while not CONFIGURED and have no live hardware state to
mismatch.

Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>
---
 drivers/block/null_blk/main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index cab51301560e..e7555c47b671 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -360,13 +360,15 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page,	\
 	ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
 	if (ret < 0)							\
 		return ret;						\
-	if (apply_fn)							\
+	if (apply_fn) {							\
 		ret = apply_fn(dev, new_value);				\
-	else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) 	\
-		ret = -EBUSY;						\
-	if (ret < 0)							\
-		return ret;						\
-	dev->NAME = new_value;						\
+		if (ret < 0)						\
+			return ret;					\
+	} else {							\
+		if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags))	\
+			return -EBUSY;					\
+		dev->NAME = new_value;					\
+	}								\
 	return count;							\
 }									\
 CONFIGFS_ATTR(nullb_device_, NAME);
-- 
2.52.0