| Message ID | 1459764020-126038-2-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/04/2016 03:00 AM, Hannes Reinecke wrote: > The queue limits already have a 'chunk_sectors' setting, so > we should be presenting it via sysfs. This patch does more than exporting chunk_sectors via sysfs. It also makes that parameter configurable. Please mention this in the patch description. My understanding of the block drivers that call blk_queue_chunk_sectors() is that increasing the chunk_sectors parameter will break these drivers. I think the single queue_limits.chunk_sectors parameter needs to be converted into two parameters: - The value set by the block driver by calling blk_queue_chunk_sectors(). - The value configured from user space through sysfs. This will allow to ensure that the chunk_sectors parameter can be increased from user space and also that it cannot be decreased. > +static ssize_t > +queue_chunk_sectors_store(struct request_queue *q, const char *page, size_t count) > +{ > + unsigned long chunk_sectors; > + > + ssize_t ret = queue_var_store(&chunk_sectors, page, count); > + if (ret < 0) > + return ret; > + spin_lock_irq(q->queue_lock); > + blk_queue_chunk_sectors(q, chunk_sectors); > + spin_unlock_irq(q->queue_lock); > + > + return ret; > +} In blk_queue_chunk_sectors() I found the following: BUG_ON(!is_power_of_2(chunk_sectors)); Please make sure that this BUG_ON() cannot be triggered from user space. Additionally, an update for Documentation/block/queue-sysfs.txt is missing from this patch. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/14/2016 09:09 PM, Bart Van Assche wrote: > On 04/04/2016 03:00 AM, Hannes Reinecke wrote: >> The queue limits already have a 'chunk_sectors' setting, so >> we should be presenting it via sysfs. > > This patch does more than exporting chunk_sectors via sysfs. It also > makes that parameter configurable. Please mention this in the patch > description. > > My understanding of the block drivers that call > blk_queue_chunk_sectors() is that increasing the chunk_sectors > parameter will break these drivers. I think the single > queue_limits.chunk_sectors parameter needs to be converted into two > parameters: > - The value set by the block driver by calling > blk_queue_chunk_sectors(). > - The value configured from user space through sysfs. > > This will allow to ensure that the chunk_sectors parameter can be > increased from user space and also that it cannot be decreased. > >> +static ssize_t >> +queue_chunk_sectors_store(struct request_queue *q, const char >> *page, size_t count) >> +{ >> + unsigned long chunk_sectors; >> + >> + ssize_t ret = queue_var_store(&chunk_sectors, page, count); >> + if (ret < 0) >> + return ret; >> + spin_lock_irq(q->queue_lock); >> + blk_queue_chunk_sectors(q, chunk_sectors); >> + spin_unlock_irq(q->queue_lock); >> + >> + return ret; >> +} > > In blk_queue_chunk_sectors() I found the following: > > BUG_ON(!is_power_of_2(chunk_sectors)); > > Please make sure that this BUG_ON() cannot be triggered from user > space. > > Additionally, an update for Documentation/block/queue-sysfs.txt is > missing from this patch. > Ah, right. Actually, making the sysfs attribute writeable is a leftover from the original (debugging) patchset, so I'll be removing it from the next iteration of this patchset. Cheers, Hannes
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index dd93763..ff97091 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -130,6 +130,26 @@ static ssize_t queue_physical_block_size_show(struct request_queue *q, char *pag return queue_var_show(queue_physical_block_size(q), page); } +static ssize_t queue_chunk_sectors_show(struct request_queue *q, char *page) +{ + return queue_var_show(q->limits.chunk_sectors, page); +} + +static ssize_t +queue_chunk_sectors_store(struct request_queue *q, const char *page, size_t count) +{ + unsigned long chunk_sectors; + + ssize_t ret = queue_var_store(&chunk_sectors, page, count); + if (ret < 0) + return ret; + spin_lock_irq(q->queue_lock); + blk_queue_chunk_sectors(q, chunk_sectors); + spin_unlock_irq(q->queue_lock); + + return ret; +} + static ssize_t queue_io_min_show(struct request_queue *q, char *page) { return queue_var_show(queue_io_min(q), page); @@ -406,6 +426,12 @@ static struct queue_sysfs_entry queue_physical_block_size_entry = { .show = queue_physical_block_size_show, }; +static struct queue_sysfs_entry queue_chunk_sectors_entry = { + .attr = {.name = "chunk_sectors", .mode = S_IRUGO | S_IWUSR }, + .show = queue_chunk_sectors_show, + .store = queue_chunk_sectors_store, +}; + static struct queue_sysfs_entry queue_io_min_entry = { .attr = {.name = "minimum_io_size", .mode = S_IRUGO }, .show = queue_io_min_show, @@ -490,6 +516,7 @@ static struct attribute *default_attrs[] = { &queue_hw_sector_size_entry.attr, &queue_logical_block_size_entry.attr, &queue_physical_block_size_entry.attr, + &queue_chunk_sectors_entry.attr, &queue_io_min_entry.attr, &queue_io_opt_entry.attr, &queue_discard_granularity_entry.attr,
The queue limits already have a 'chunk_sectors' setting, so we should be presenting it via sysfs. Signed-off-by: Hannes Reinecke <hare@suse.de> --- block/blk-sysfs.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)