On 10/5/22 09:54, Bart Van Assche wrote: > On 10/5/22 02:47, Ulf Hansson wrote: >> On Wed, 5 Oct 2022 at 07:11, Damien Le Moal >> <damien.lemoal@xxxxxxxxxxxxxxxxxx> 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. > Thanks for the feedback, will drop it. -ck
