On 12/25/24 7:09 PM, Shin'ichiro Kawasaki wrote:
> The null_blk configfs file 'features' provides a string that lists
> available null_blk features for userspace programs to reference.
> The string is defined as a long constant in the code, which tends to be
> forgotten for updates. It also causes checkpatch.pl to report
> "WARNING: quoted string split across lines".
>
> To avoid these drawbacks, generate the feature string on the fly. Refer
> to the ca_name field of each element in the nullb_device_attrs table and
> concatenate them in the given buffer. Also, modify order of the
> nullb_device_attrs table to not change the list order of the generated
> string.
>
> Of note is that the feature "index" was missing before this commit.
> This commit adds it to the generated string.
>
> Suggested-by: Bart Van Assche <bvanassche@xxxxxxx>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
Nice cleanup ! One nit below. And with that fixed, feel free to add:
Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> @@ -704,16 +708,27 @@ 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");
> +
> + struct configfs_attribute **entry;
> + const char *fmt = "%s,";
> + size_t left = PAGE_SIZE;
> + size_t written = 0;
> + int ret;
> +
> + for (entry = &nullb_device_attrs[0]; *entry && left > 0; entry++) {
> + if (!*(entry + 1))
> + fmt = "%s\n";
> + ret = snprintf(page + written, left, fmt, (*entry)->ca_name);
> + if (ret >= left) {
> + WARN_ONCE(1, "Too many null_blk features to print\n");
> + memzero_explicit(page, PAGE_SIZE);
> + return 0;
Let's return an error here. Maybe "-EFBIG" ?
Note that we are nowhere near to hit this though, nor should we ever reach a 4K
string length for null_blk features :)
> + }
> + left -= ret;
> + written += ret;
> + }
> +
> + return written;
> }
>
> CONFIGFS_ATTR_RO(memb_group_, features);
--
Damien Le Moal
Western Digital Research
