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

[Bart Van Assche]

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.