Re: [PATCH v2 25/28] block: Move zone related debugfs attribute to blk-zoned.c

Damien Le Moal
2024-03-25T23:17:42Z

On 3/26/24 07:20, Bart Van Assche wrote:
> On 3/24/24 21:44, Damien Le Moal wrote:
>> diff --git a/block/blk-mq-debugfs.h b/block/blk-mq-debugfs.h
>> index 9c7d4b6117d4..3ebe2c29b624 100644
>> --- a/block/blk-mq-debugfs.h
>> +++ b/block/blk-mq-debugfs.h
>> @@ -83,7 +83,7 @@ static inline void blk_mq_debugfs_unregister_rqos(struct rq_qos *rqos)
>> }
>> #endif
>>
>> -#ifdef CONFIG_BLK_DEBUG_FS_ZONED
>> +#if defined(CONFIG_BLK_DEV_ZONED) && defined(CONFIG_BLK_DEBUG_FS)
>> int queue_zone_wlock_show(void *data, struct seq_file *m);
>> #else
>> static inline int queue_zone_wlock_show(void *data, struct seq_file *m)
>> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>> index 03222314d649..62160a8675f4 100644
>> --- a/block/blk-zoned.c
>> +++ b/block/blk-zoned.c
>> @@ -22,6 +22,7 @@
>>
>> #include "blk.h"
>> #include "blk-mq-sched.h"
>> +#include "blk-mq-debugfs.h"
>>
>> #define ZONE_COND_NAME(name) [BLK_ZONE_COND_##name] = #name
>> static const char *const zone_cond_name[] = {
>> @@ -1745,3 +1746,22 @@ void blk_zone_dev_init(void)
>> {
>> blk_zone_wplugs_cachep = KMEM_CACHE(blk_zone_wplug, SLAB_PANIC);
>> }
>> +
>> +#ifdef CONFIG_BLK_DEBUG_FS
>> +
>> +int queue_zone_wlock_show(void *data, struct seq_file *m)
>> +{
>> + struct request_queue *q = data;
>> + unsigned int i;
>> +
>> + if (!q->disk->seq_zones_wlock)
>> + return 0;
>> +
>> + for (i = 0; i < q->disk->nr_zones; i++)
>> + if (test_bit(i, q->disk->seq_zones_wlock))
>> + seq_printf(m, "%u\n", i);
>> +
>> + return 0;
>> +}
>> +
>> +#endif
>
> This patch increases the number of #ifdefs in block layer .c files so
> I'm not sure that this patch can be considered an improvement.

Not following... This removes CONFIG_BLK_DEBUG_FS_ZONED and only add this single
ifdef here in blk-zoned.c instead of testing the removed config in
blk-mq-debugfs.h. Same count and one less config. Sure, there is one ifdef in c
code added. But I *really* prefer that instead of having zone related code all
over the place guarded with #if defined(CONFIG_BLK_DEV_ZONED).

So unless Jens is against this change, I am keeping it.

>
> Thanks,
>
> Bart.

--
Damien Le Moal
Western Digital Research