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

Bart Van Assche
2022-10-06T06:28:02Z

On 10/5/22 02:47, Ulf Hansson wrote:
> On Wed, 5 Oct 2022 at 07:11, Damien Le Moal <damien.lemoal@opensource.wdc.com> wrote:
>> On 10/5/22 12:22, Chaitanya Kulkarni wrote:
>>> +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.
>> Using a function with that many arguments will be error prone, and hard to
>> review... Not a fan.
>
> I completely agree.
>
> But there is also another problem going down this route. If/when we
> realize that there is another parameter needed in the blk_mq_tag_set.
> Today that's quite easy to add (assuming the parameter can be
> optional), without changing the blk_mq_init_tag_set() interface.

Hi Chaitanya,

Please consider to drop the entire patch series. In addition to the
disadvantages mentioned above I'd like to mention the following
disadvantages:
* Replacing named member assignments with positional arguments in a
function call makes code harder to read and harder to verify.
* This patch series makes tree-wide changes without improving the code
in a substantial way.

Thanks,

Bart.