block: add a store_limit operations for sysfs entries

De-duplicate the code for updating queue limits by adding a store_limit
method that allows having common code handle the actual queue limits
update.

Note that this is a pure refactoring patch and does not address the
existing freeze vs limits lock order problem in the refactored code,
which will be addressed next.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Link: https://lore.kernel.org/r/20250110054726.1499538-6-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index e9f1c82b2f3e37a0d822d6c74bf00dec1df46592..d2aa2177e4ba5a2fba1d142892f6b3cc7576e16b 100644 (file)
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -24,6 +24,8 @@ struct queue_sysfs_entry {
        struct attribute attr;
        ssize_t (*show)(struct gendisk *disk, char *page);
        ssize_t (*store)(struct gendisk *disk, const char *page, size_t count);
+       int (*store_limit)(struct gendisk *disk, const char *page,
+                       size_t count, struct queue_limits *lim);
        void (*load_module)(struct gendisk *disk, const char *page, size_t count);
 };
 
@@ -153,13 +155,11 @@ QUEUE_SYSFS_SHOW_CONST(discard_zeroes_data, 0)
 QUEUE_SYSFS_SHOW_CONST(write_same_max, 0)
 QUEUE_SYSFS_SHOW_CONST(poll_delay, -1)
 
-static ssize_t queue_max_discard_sectors_store(struct gendisk *disk,
-               const char *page, size_t count)
+static int queue_max_discard_sectors_store(struct gendisk *disk,
+               const char *page, size_t count, struct queue_limits *lim)
 {
        unsigned long max_discard_bytes;
-       struct queue_limits lim;
        ssize_t ret;
-       int err;
 
        ret = queue_var_store(&max_discard_bytes, page, count);
        if (ret < 0)
@@ -171,38 +171,28 @@ static ssize_t queue_max_discard_sectors_store(struct gendisk *disk,
        if ((max_discard_bytes >> SECTOR_SHIFT) > UINT_MAX)
                return -EINVAL;
 
-       lim = queue_limits_start_update(disk->queue);
-       lim.max_user_discard_sectors = max_discard_bytes >> SECTOR_SHIFT;
-       err = queue_limits_commit_update(disk->queue, &lim);
-       if (err)
-               return err;
-       return ret;
+       lim->max_user_discard_sectors = max_discard_bytes >> SECTOR_SHIFT;
+       return 0;
 }
 
-static ssize_t
-queue_max_sectors_store(struct gendisk *disk, const char *page, size_t count)
+static int
+queue_max_sectors_store(struct gendisk *disk, const char *page, size_t count,
+               struct queue_limits *lim)
 {
        unsigned long max_sectors_kb;
-       struct queue_limits lim;
        ssize_t ret;
-       int err;
 
        ret = queue_var_store(&max_sectors_kb, page, count);
        if (ret < 0)
                return ret;
 
-       lim = queue_limits_start_update(disk->queue);
-       lim.max_user_sectors = max_sectors_kb << 1;
-       err = queue_limits_commit_update(disk->queue, &lim);
-       if (err)
-               return err;
-       return ret;
+       lim->max_user_sectors = max_sectors_kb << 1;
+       return 0;
 }
 
 static ssize_t queue_feature_store(struct gendisk *disk, const char *page,
-               size_t count, blk_features_t feature)
+               size_t count, struct queue_limits *lim, blk_features_t feature)
 {
-       struct queue_limits lim;
        unsigned long val;
        ssize_t ret;
 
@@ -210,15 +200,11 @@ static ssize_t queue_feature_store(struct gendisk *disk, const char *page,
        if (ret < 0)
                return ret;
 
-       lim = queue_limits_start_update(disk->queue);
        if (val)
-               lim.features |= feature;
+               lim->features |= feature;
        else
-               lim.features &= ~feature;
-       ret = queue_limits_commit_update(disk->queue, &lim);
-       if (ret)
-               return ret;
-       return count;
+               lim->features &= ~feature;
+       return 0;
 }
 
 #define QUEUE_SYSFS_FEATURE(_name, _feature)                           \
@@ -227,10 +213,10 @@ static ssize_t queue_##_name##_show(struct gendisk *disk, char *page)     \
        return sysfs_emit(page, "%u\n",                                 \
                !!(disk->queue->limits.features & _feature));           \
 }                                                                      \
-static ssize_t queue_##_name##_store(struct gendisk *disk,             \
-               const char *page, size_t count)                         \
+static int queue_##_name##_store(struct gendisk *disk,                 \
+               const char *page, size_t count, struct queue_limits *lim) \
 {                                                                      \
-       return queue_feature_store(disk, page, count, _feature);        \
+       return queue_feature_store(disk, page, count, lim, _feature);   \
 }
 
 QUEUE_SYSFS_FEATURE(rotational, BLK_FEAT_ROTATIONAL)
@@ -273,10 +259,9 @@ static ssize_t queue_iostats_passthrough_show(struct gendisk *disk, char *page)
        return queue_var_show(!!blk_queue_passthrough_stat(disk->queue), page);
 }
 
-static ssize_t queue_iostats_passthrough_store(struct gendisk *disk,
-                                              const char *page, size_t count)
+static int queue_iostats_passthrough_store(struct gendisk *disk,
+               const char *page, size_t count, struct queue_limits *lim)
 {
-       struct queue_limits lim;
        unsigned long ios;
        ssize_t ret;
 
@@ -284,18 +269,13 @@ static ssize_t queue_iostats_passthrough_store(struct gendisk *disk,
        if (ret < 0)
                return ret;
 
-       lim = queue_limits_start_update(disk->queue);
        if (ios)
-               lim.flags |= BLK_FLAG_IOSTATS_PASSTHROUGH;
+               lim->flags |= BLK_FLAG_IOSTATS_PASSTHROUGH;
        else
-               lim.flags &= ~BLK_FLAG_IOSTATS_PASSTHROUGH;
-
-       ret = queue_limits_commit_update(disk->queue, &lim);
-       if (ret)
-               return ret;
-
-       return count;
+               lim->flags &= ~BLK_FLAG_IOSTATS_PASSTHROUGH;
+       return 0;
 }
+
 static ssize_t queue_nomerges_show(struct gendisk *disk, char *page)
 {
        return queue_var_show((blk_queue_nomerges(disk->queue) << 1) |
@@ -398,12 +378,10 @@ static ssize_t queue_wc_show(struct gendisk *disk, char *page)
        return sysfs_emit(page, "write through\n");
 }
 
-static ssize_t queue_wc_store(struct gendisk *disk, const char *page,
-                             size_t count)
+static int queue_wc_store(struct gendisk *disk, const char *page,
+               size_t count, struct queue_limits *lim)
 {
-       struct queue_limits lim;
        bool disable;
-       int err;
 
        if (!strncmp(page, "write back", 10)) {
                disable = false;
@@ -414,15 +392,11 @@ static ssize_t queue_wc_store(struct gendisk *disk, const char *page,
                return -EINVAL;
        }
 
-       lim = queue_limits_start_update(disk->queue);
        if (disable)
-               lim.flags |= BLK_FLAG_WRITE_CACHE_DISABLED;
+               lim->flags |= BLK_FLAG_WRITE_CACHE_DISABLED;
        else
-               lim.flags &= ~BLK_FLAG_WRITE_CACHE_DISABLED;
-       err = queue_limits_commit_update(disk->queue, &lim);
-       if (err)
-               return err;
-       return count;
+               lim->flags &= ~BLK_FLAG_WRITE_CACHE_DISABLED;
+       return 0;
 }
 
 #define QUEUE_RO_ENTRY(_prefix, _name)                 \
@@ -438,6 +412,13 @@ static struct queue_sysfs_entry _prefix##_entry = {        \
        .store  = _prefix##_store,                      \
 };
 
+#define QUEUE_LIM_RW_ENTRY(_prefix, _name)                     \
+static struct queue_sysfs_entry _prefix##_entry = {    \
+       .attr           = { .name = _name, .mode = 0644 },      \
+       .show           = _prefix##_show,                       \
+       .store_limit    = _prefix##_store,                      \
+}
+
 #define QUEUE_RW_LOAD_MODULE_ENTRY(_prefix, _name)             \
 static struct queue_sysfs_entry _prefix##_entry = {            \
        .attr           = { .name = _name, .mode = 0644 },      \
@@ -448,7 +429,7 @@ static struct queue_sysfs_entry _prefix##_entry = {         \
 
 QUEUE_RW_ENTRY(queue_requests, "nr_requests");
 QUEUE_RW_ENTRY(queue_ra, "read_ahead_kb");
-QUEUE_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
+QUEUE_LIM_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
 QUEUE_RO_ENTRY(queue_max_hw_sectors, "max_hw_sectors_kb");
 QUEUE_RO_ENTRY(queue_max_segments, "max_segments");
 QUEUE_RO_ENTRY(queue_max_integrity_segments, "max_integrity_segments");
@@ -464,7 +445,7 @@ QUEUE_RO_ENTRY(queue_io_opt, "optimal_io_size");
 QUEUE_RO_ENTRY(queue_max_discard_segments, "max_discard_segments");
 QUEUE_RO_ENTRY(queue_discard_granularity, "discard_granularity");
 QUEUE_RO_ENTRY(queue_max_hw_discard_sectors, "discard_max_hw_bytes");
-QUEUE_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
+QUEUE_LIM_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
 QUEUE_RO_ENTRY(queue_discard_zeroes_data, "discard_zeroes_data");
 
 QUEUE_RO_ENTRY(queue_atomic_write_max_sectors, "atomic_write_max_bytes");
@@ -484,11 +465,11 @@ QUEUE_RO_ENTRY(queue_max_open_zones, "max_open_zones");
 QUEUE_RO_ENTRY(queue_max_active_zones, "max_active_zones");
 
 QUEUE_RW_ENTRY(queue_nomerges, "nomerges");
-QUEUE_RW_ENTRY(queue_iostats_passthrough, "iostats_passthrough");
+QUEUE_LIM_RW_ENTRY(queue_iostats_passthrough, "iostats_passthrough");
 QUEUE_RW_ENTRY(queue_rq_affinity, "rq_affinity");
 QUEUE_RW_ENTRY(queue_poll, "io_poll");
 QUEUE_RW_ENTRY(queue_poll_delay, "io_poll_delay");
-QUEUE_RW_ENTRY(queue_wc, "write_cache");
+QUEUE_LIM_RW_ENTRY(queue_wc, "write_cache");
 QUEUE_RO_ENTRY(queue_fua, "fua");
 QUEUE_RO_ENTRY(queue_dax, "dax");
 QUEUE_RW_ENTRY(queue_io_timeout, "io_timeout");
@@ -501,10 +482,10 @@ static struct queue_sysfs_entry queue_hw_sector_size_entry = {
        .show = queue_logical_block_size_show,
 };
 
-QUEUE_RW_ENTRY(queue_rotational, "rotational");
-QUEUE_RW_ENTRY(queue_iostats, "iostats");
-QUEUE_RW_ENTRY(queue_add_random, "add_random");
-QUEUE_RW_ENTRY(queue_stable_writes, "stable_writes");
+QUEUE_LIM_RW_ENTRY(queue_rotational, "rotational");
+QUEUE_LIM_RW_ENTRY(queue_iostats, "iostats");
+QUEUE_LIM_RW_ENTRY(queue_add_random, "add_random");
+QUEUE_LIM_RW_ENTRY(queue_stable_writes, "stable_writes");
 
 #ifdef CONFIG_BLK_WBT
 static ssize_t queue_var_store64(s64 *var, const char *page)
@@ -702,7 +683,7 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
        struct request_queue *q = disk->queue;
        ssize_t res;
 
-       if (!entry->store)
+       if (!entry->store_limit && !entry->store)
                return -EIO;
 
        /*
@@ -713,11 +694,24 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
        if (entry->load_module)
                entry->load_module(disk, page, length);
 
-       blk_mq_freeze_queue(q);
        mutex_lock(&q->sysfs_lock);
-       res = entry->store(disk, page, length);
-       mutex_unlock(&q->sysfs_lock);
+       blk_mq_freeze_queue(q);
+       if (entry->store_limit) {
+               struct queue_limits lim = queue_limits_start_update(q);
+
+               res = entry->store_limit(disk, page, length, &lim);
+               if (res < 0) {
+                       queue_limits_cancel_update(q);
+               } else {
+                       res = queue_limits_commit_update(q, &lim);
+                       if (!res)
+                               res = length;
+               }
+       } else {
+               res = entry->store(disk, page, length);
+       }
        blk_mq_unfreeze_queue(q);
+       mutex_unlock(&q->sysfs_lock);
        return res;
 }