| Message ID | 20241218074914.814913-4-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | null_blk: improve write failure simulation | expand |
On 12/17/24 11:49 PM, Shin'ichiro Kawasaki wrote: > static ssize_t memb_group_features_show(struct config_item *item, char *page) > { > return snprintf(page, PAGE_SIZE, > - "badblocks,blocking,blocksize,cache_size,fua," > - "completion_nsec,discard,home_node,hw_queue_depth," > - "irqmode,max_sectors,mbps,memory_backed,no_sched," > - "poll_queues,power,queue_mode,shared_tag_bitmap," > - "shared_tags,size,submit_queues,use_per_node_hctx," > - "virt_boundary,zoned,zone_capacity,zone_max_active," > - "zone_max_open,zone_nr_conv,zone_offline,zone_readonly," > - "zone_size,zone_append_max_sectors,zone_full," > - "rotational\n"); > + "badblocks,badblocks_once,blocking,blocksize," > + "cache_size,completion_nsec," > + "discard," > + "fua," > + "home_node,hw_queue_depth," > + "irqmode," > + "max_sectors,mbps,memory_backed," > + "no_sched," > + "poll_queues,power," > + "queue_mode," > + "rotational," > + "shared_tag_bitmap,shared_tags,size,submit_queues," > + "use_per_node_hctx," > + "virt_boundary," > + "zoned,zone_capacity,zone_max_active,zone_max_open," > + "zone_nr_conv,zone_offline,zone_readonly,zone_size," > + "zone_append_max_sectors,zone_full\n"); > } The entire list of attributes occurs twice in the null_blk source code. This is error prone. Has it been considered to modify memb_group_features_show() such that it iterates over the nullb_device_attrs array instead of duplicating information that already exists in that array? Thanks, Bart.
On Dec 18, 2024 / 09:27, Bart Van Assche wrote: > On 12/17/24 11:49 PM, Shin'ichiro Kawasaki wrote: > > static ssize_t memb_group_features_show(struct config_item *item, char *page) > > { > > return snprintf(page, PAGE_SIZE, > > - "badblocks,blocking,blocksize,cache_size,fua," > > - "completion_nsec,discard,home_node,hw_queue_depth," > > - "irqmode,max_sectors,mbps,memory_backed,no_sched," > > - "poll_queues,power,queue_mode,shared_tag_bitmap," > > - "shared_tags,size,submit_queues,use_per_node_hctx," > > - "virt_boundary,zoned,zone_capacity,zone_max_active," > > - "zone_max_open,zone_nr_conv,zone_offline,zone_readonly," > > - "zone_size,zone_append_max_sectors,zone_full," > > - "rotational\n"); > > + "badblocks,badblocks_once,blocking,blocksize," > > + "cache_size,completion_nsec," > > + "discard," > > + "fua," > > + "home_node,hw_queue_depth," > > + "irqmode," > > + "max_sectors,mbps,memory_backed," > > + "no_sched," > > + "poll_queues,power," > > + "queue_mode," > > + "rotational," > > + "shared_tag_bitmap,shared_tags,size,submit_queues," > > + "use_per_node_hctx," > > + "virt_boundary," > > + "zoned,zone_capacity,zone_max_active,zone_max_open," > > + "zone_nr_conv,zone_offline,zone_readonly,zone_size," > > + "zone_append_max_sectors,zone_full\n"); > > } > > The entire list of attributes occurs twice in the null_blk source code. > This is error prone. Has it been considered to modify > memb_group_features_show() such that it iterates over the > nullb_device_attrs array instead of duplicating information that already > exists in that array? This sounds a good idea. Will create a patch for it and add it to v2.
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index 0f02e763cd9e..f5dd25fd1bbf 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -473,6 +473,7 @@ NULLB_DEVICE_ATTR(shared_tags, bool, NULL); NULLB_DEVICE_ATTR(shared_tag_bitmap, bool, NULL); NULLB_DEVICE_ATTR(fua, bool, NULL); NULLB_DEVICE_ATTR(rotational, bool, NULL); +NULLB_DEVICE_ATTR(badblocks_once, bool, NULL); static ssize_t nullb_device_power_show(struct config_item *item, char *page) { @@ -611,6 +612,7 @@ static struct configfs_attribute *nullb_device_attrs[] = { &nullb_device_attr_mbps, &nullb_device_attr_cache_size, &nullb_device_attr_badblocks, + &nullb_device_attr_badblocks_once, &nullb_device_attr_zoned, &nullb_device_attr_zone_size, &nullb_device_attr_zone_capacity, @@ -705,15 +707,23 @@ nullb_group_drop_item(struct config_group *group, struct config_item *item) static ssize_t memb_group_features_show(struct config_item *item, char *page) { return snprintf(page, PAGE_SIZE, - "badblocks,blocking,blocksize,cache_size,fua," - "completion_nsec,discard,home_node,hw_queue_depth," - "irqmode,max_sectors,mbps,memory_backed,no_sched," - "poll_queues,power,queue_mode,shared_tag_bitmap," - "shared_tags,size,submit_queues,use_per_node_hctx," - "virt_boundary,zoned,zone_capacity,zone_max_active," - "zone_max_open,zone_nr_conv,zone_offline,zone_readonly," - "zone_size,zone_append_max_sectors,zone_full," - "rotational\n"); + "badblocks,badblocks_once,blocking,blocksize," + "cache_size,completion_nsec," + "discard," + "fua," + "home_node,hw_queue_depth," + "irqmode," + "max_sectors,mbps,memory_backed," + "no_sched," + "poll_queues,power," + "queue_mode," + "rotational," + "shared_tag_bitmap,shared_tags,size,submit_queues," + "use_per_node_hctx," + "virt_boundary," + "zoned,zone_capacity,zone_max_active,zone_max_open," + "zone_nr_conv,zone_offline,zone_readonly,zone_size," + "zone_append_max_sectors,zone_full\n"); } CONFIGFS_ATTR_RO(memb_group_, features); @@ -1327,6 +1337,8 @@ static inline blk_status_t null_handle_badblocks(struct nullb_cmd *cmd, int bad_sectors; if (badblocks_check(bb, sector, nr_sectors, &first_bad, &bad_sectors)) { + if (cmd->nq->dev->badblocks_once) + badblocks_clear(bb, first_bad, bad_sectors); if (!IS_ALIGNED(first_bad, block_sectors)) first_bad = ALIGN_DOWN(first_bad, block_sectors); if (dev->memory_backed && sector < first_bad) { diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h index c6ceede691ba..b9cd85542498 100644 --- a/drivers/block/null_blk/null_blk.h +++ b/drivers/block/null_blk/null_blk.h @@ -63,6 +63,7 @@ struct nullb_device { unsigned long flags; /* device flags */ unsigned int curr_cache; struct badblocks badblocks; + bool badblocks_once; unsigned int nr_zones; unsigned int nr_zones_imp_open;
When IO errors happen on real storage devices, the IOs repeated to the same target range can success by virtue of recovery features by devices, such as reserved block assignment. To simulate such IO errors and recoveries, introduce the new parameter badblocks_once parameter. When this parameter is set to 1, the specified badblocks are cleared after the first IO error, so that the next IO to the blocks succeed. While at it, split the long string constant in memb_group_features_show() into multiple lines to make future changes easier to understand. Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- drivers/block/null_blk/main.c | 30 +++++++++++++++++++++--------- drivers/block/null_blk/null_blk.h | 1 + 2 files changed, 22 insertions(+), 9 deletions(-)