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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e0ea0b0a-5077-de37-046f-62902aca93b6@acm.org>
Date:   Wed, 5 Oct 2022 09:54:11 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Chaitanya Kulkarni <kch@...dia.com>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        Ulf Hansson <ulf.hansson@...aro.org>
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-omap@...r.kernel.org, linux-remoteproc@...r.kernel.org,
        linux-mmc@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-nvme@...ts.infradead.org, linux-s390@...r.kernel.org,
        linux-scsi@...r.kernel.org, axboe@...nel.dk, efremov@...ux.com,
        josef@...icpanda.com, idryomov@...il.com,
        dongsheng.yang@...ystack.cn, haris.iqbal@...os.com,
        jinpu.wang@...os.com, mst@...hat.com, jasowang@...hat.com,
        pbonzini@...hat.com, stefanha@...hat.com, ohad@...ery.com,
        andersson@...nel.org, baolin.wang@...ux.alibaba.com,
        richard@....at, miquel.raynal@...tlin.com, vigneshr@...com,
        marcan@...can.st, sven@...npeter.dev, alyssa@...enzweig.io,
        kbusch@...nel.org, hch@....de, sagi@...mberg.me, sth@...ux.ibm.com,
        hoeppner@...ux.ibm.com, hca@...ux.ibm.com, gor@...ux.ibm.com,
        agordeev@...ux.ibm.com, borntraeger@...ux.ibm.com,
        svens@...ux.ibm.com, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, hare@...e.de, bhelgaas@...gle.com,
        john.garry@...wei.com, mcgrof@...nel.org,
        christophe.jaillet@...adoo.fr, vaibhavgupta40@...il.com,
        wsa+renesas@...g-engineering.com, johannes.thumshirn@....com,
        ming.lei@...hat.com, shinichiro.kawasaki@....com,
        vincent.fu@...sung.com, christoph.boehmwalder@...bit.com,
        joel@....id.au, vincent.whitchurch@...s.com, nbd@...er.debian.org,
        ceph-devel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, asahi@...ts.linux.dev
Subject: Re: [RFC PATCH 01/21] block: add and use init tagset helper

On 10/5/22 02:47, Ulf Hansson wrote:
> On Wed, 5 Oct 2022 at 07:11, Damien Le Moal <damien.lemoal@...nsource.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.