Re: [RFC PATCH 01/21] block: add and use init tagset helper

[Chaitanya Kulkarni]

On 10/4/22 22:11, Damien Le Moal wrote:
> On 10/5/22 12:22, Chaitanya Kulkarni wrote:
>> Add and use the helper to initialize the common fields of the tag_set
>> such as blk_mq_ops, number of h/w queues, queue depth, command size,
>> numa_node, timeout, BLK_MQ_F_XXX flags, driver data. This initialization
>> is spread all over the block drivers. This avoids the code repetation of
>> the inialization code of the tag set in current block drivers and any
> 
> s/inialization/initialization
> s/repetation/repetition
> 
>> future ones.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
>> ---
>>   block/blk-mq.c                | 20 ++++++++++++++++++++
>>   drivers/block/null_blk/main.c | 10 +++-------
>>   include/linux/blk-mq.h        |  5 +++++
>>   3 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 8070b6c10e8d..e3a8dd81bbe2 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -4341,6 +4341,26 @@ static int blk_mq_alloc_tag_set_tags(struct blk_mq_tag_set *set,
>>   	return blk_mq_realloc_tag_set_tags(set, 0, new_nr_hw_queues);
>>   }
>>   
>> +void blk_mq_init_tag_set(struct blk_mq_tag_set *set,
>> +		const struct blk_mq_ops *ops, unsigned int nr_hw_queues,
>> +		unsigned int queue_depth, unsigned int cmd_size, int numa_node,
>> +		unsigned int timeout, unsigned int flags, void *driver_data)
> 
> That is an awful lot of arguments... I would be tempted to say pack all
> these into a struct but then that would kind of negate this patchset goal.
yes..

> Using a function with that many arguments will be error prone, and hard to
> review... Not a fan.
> 

Recent addition to the block layer code blk_rq_map_user_io() has 9 
arguments:-
int blk_rq_map_user_io(struct request *req, struct rq_map_data
*map_data, void __user *ubuf, unsigned long buf_len,
gfp_t gfp_mask, bool vec, int iov_count, bool check_iter_count,
int rw)

above function also has 9 arguments not more than what is present
in the block tree. I can trim down the argument list probably by
removing the numa_node as it is set to NUMA_NO_NODE for most of the
drivers.

-ck