Message ID | 20230105205146.3610282-3-kbusch@meta.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | block: don't forget user settings | expand |
On 1/5/23 12:51, Keith Busch wrote: > diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c > index 93d9e9c9a6ea8..5486b6c57f6b8 100644 > --- a/block/blk-sysfs.c > +++ b/block/blk-sysfs.c > @@ -239,19 +239,28 @@ static ssize_t queue_zone_append_max_show(struct request_queue *q, char *page) > static ssize_t > queue_max_sectors_store(struct request_queue *q, const char *page, size_t count) > { > - unsigned long max_sectors_kb, > + unsigned long var; > + unsigned int max_sectors_kb, > max_hw_sectors_kb = queue_max_hw_sectors(q) >> 1, > page_kb = 1 << (PAGE_SHIFT - 10); > - ssize_t ret = queue_var_store(&max_sectors_kb, page, count); > + ssize_t ret = queue_var_store(&var, page, count); > > if (ret < 0) > return ret; > > - max_hw_sectors_kb = min_not_zero(max_hw_sectors_kb, (unsigned long) > + max_sectors_kb = (unsigned int)var; Shouldn't this function report an error if 'var' is too large to fit into an unsigned int? Otherwise this patch looks good to me. Thanks, Bart.
On Thu, Jan 05, 2023 at 01:28:35PM -0800, Bart Van Assche wrote: > On 1/5/23 12:51, Keith Busch wrote: > > diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c > > index 93d9e9c9a6ea8..5486b6c57f6b8 100644 > > --- a/block/blk-sysfs.c > > +++ b/block/blk-sysfs.c > > @@ -239,19 +239,28 @@ static ssize_t queue_zone_append_max_show(struct request_queue *q, char *page) > > static ssize_t > > queue_max_sectors_store(struct request_queue *q, const char *page, size_t count) > > { > > - unsigned long max_sectors_kb, > > + unsigned long var; > > + unsigned int max_sectors_kb, > > max_hw_sectors_kb = queue_max_hw_sectors(q) >> 1, > > page_kb = 1 << (PAGE_SHIFT - 10); > > - ssize_t ret = queue_var_store(&max_sectors_kb, page, count); > > + ssize_t ret = queue_var_store(&var, page, count); > > if (ret < 0) > > return ret; > > - max_hw_sectors_kb = min_not_zero(max_hw_sectors_kb, (unsigned long) > > + max_sectors_kb = (unsigned int)var; > > Shouldn't this function report an error if 'var' is too large to fit into an > unsigned int? Yes it should, and queue_var_store() will return -EINVAL if that happens. That's certainly not clear just from reading this patch, but the condition is handled.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Keith, > The user can set the max_sectors limit to any valid value via sysfs > /sys/block/<dev>/queue/max_sectors_kb attribute. If the device limits > are ever rescanned, though, the limit reverts back to the potentially > artificially low BLK_DEF_MAX_SECTORS value. > > Preserve the user's setting as the max_sectors limit as long as it's > valid. The user can reset back to defaults by writing 0 to the sysfs > file. Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block index cd14ecb3c9a5a..ac1e519272aa2 100644 --- a/Documentation/ABI/stable/sysfs-block +++ b/Documentation/ABI/stable/sysfs-block @@ -432,7 +432,8 @@ Contact: linux-block@vger.kernel.org Description: [RW] This is the maximum number of kilobytes that the block layer will allow for a filesystem request. Must be smaller than - or equal to the maximum size allowed by the hardware. + or equal to the maximum size allowed by the hardware. Write 0 + to use default kernel settings. What: /sys/block/<disk>/queue/max_segment_size diff --git a/block/blk-settings.c b/block/blk-settings.c index 9875ca131eb0c..9c9713c9269cc 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -40,7 +40,7 @@ void blk_set_default_limits(struct queue_limits *lim) lim->virt_boundary_mask = 0; lim->max_segment_size = BLK_MAX_SEGMENT_SIZE; lim->max_sectors = lim->max_hw_sectors = BLK_SAFE_MAX_SECTORS; - lim->max_dev_sectors = 0; + lim->max_user_sectors = lim->max_dev_sectors = 0; lim->chunk_sectors = 0; lim->max_write_zeroes_sectors = 0; lim->max_zone_append_sectors = 0; @@ -135,7 +135,12 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto limits->max_hw_sectors = max_hw_sectors; max_sectors = min_not_zero(max_hw_sectors, limits->max_dev_sectors); - max_sectors = min(max_sectors, BLK_DEF_MAX_SECTORS); + + if (limits->max_user_sectors) + max_sectors = min(max_sectors, limits->max_user_sectors); + else + max_sectors = min(max_sectors, BLK_DEF_MAX_SECTORS); + max_sectors = round_down(max_sectors, limits->logical_block_size >> SECTOR_SHIFT); limits->max_sectors = max_sectors; diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 93d9e9c9a6ea8..5486b6c57f6b8 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -239,19 +239,28 @@ static ssize_t queue_zone_append_max_show(struct request_queue *q, char *page) static ssize_t queue_max_sectors_store(struct request_queue *q, const char *page, size_t count) { - unsigned long max_sectors_kb, + unsigned long var; + unsigned int max_sectors_kb, max_hw_sectors_kb = queue_max_hw_sectors(q) >> 1, page_kb = 1 << (PAGE_SHIFT - 10); - ssize_t ret = queue_var_store(&max_sectors_kb, page, count); + ssize_t ret = queue_var_store(&var, page, count); if (ret < 0) return ret; - max_hw_sectors_kb = min_not_zero(max_hw_sectors_kb, (unsigned long) + max_sectors_kb = (unsigned int)var; + max_hw_sectors_kb = min_not_zero(max_hw_sectors_kb, q->limits.max_dev_sectors >> 1); - - if (max_sectors_kb > max_hw_sectors_kb || max_sectors_kb < page_kb) - return -EINVAL; + if (max_sectors_kb == 0) { + q->limits.max_user_sectors = 0; + max_sectors_kb = min(max_hw_sectors_kb, + BLK_DEF_MAX_SECTORS >> 1); + } else { + if (max_sectors_kb > max_hw_sectors_kb || + max_sectors_kb < page_kb) + return -EINVAL; + q->limits.max_user_sectors = max_sectors_kb << 1; + } spin_lock_irq(&q->queue_lock); q->limits.max_sectors = max_sectors_kb << 1; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 2b85161e22561..b87ed829ab941 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -288,6 +288,7 @@ struct queue_limits { unsigned int max_dev_sectors; unsigned int chunk_sectors; unsigned int max_sectors; + unsigned int max_user_sectors; unsigned int max_segment_size; unsigned int physical_block_size; unsigned int logical_block_size;