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.