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: <b2c5d8cd-aee1-d02f-33b4-d424a6db9c55@nvidia.com>
Date:   Wed, 5 Oct 2022 05:37:58 +0000
From:   Chaitanya Kulkarni <chaitanyak@...dia.com>
To:     Damien Le Moal <damien.lemoal@...nsource.wdc.com>
CC:     "axboe@...nel.dk" <axboe@...nel.dk>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        Keith Busch <kbusch@...nel.org>,
        Chaitanya Kulkarni <chaitanyak@...dia.com>
Subject: Re: [RFC PATCH 01/21] block: add and use init tagset helper

On 10/4/22 22:11, Damien Le Moal wrote:
> On 10/5/22 12:22, Chaitanya Kulkarni wrote:
>> Add and use the helper to initialize the common fields of the tag_set
>> such as blk_mq_ops, number of h/w queues, queue depth, command size,
>> numa_node, timeout, BLK_MQ_F_XXX flags, driver data. This initialization
>> is spread all over the block drivers. This avoids the code repetation of
>> the inialization code of the tag set in current block drivers and any
> 
> s/inialization/initialization
> s/repetation/repetition
> 
>> future ones.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@...dia.com>
>> ---
>>   block/blk-mq.c                | 20 ++++++++++++++++++++
>>   drivers/block/null_blk/main.c | 10 +++-------
>>   include/linux/blk-mq.h        |  5 +++++
>>   3 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 8070b6c10e8d..e3a8dd81bbe2 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -4341,6 +4341,26 @@ static int blk_mq_alloc_tag_set_tags(struct blk_mq_tag_set *set,
>>   	return blk_mq_realloc_tag_set_tags(set, 0, new_nr_hw_queues);
>>   }
>>   
>> +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.
yes..

> Using a function with that many arguments will be error prone, and hard to
> review... Not a fan.
> 

Recent addition to the block layer code blk_rq_map_user_io() has 9 
arguments:-
int blk_rq_map_user_io(struct request *req, struct rq_map_data
*map_data, void __user *ubuf, unsigned long buf_len,
gfp_t gfp_mask, bool vec, int iov_count, bool check_iter_count,
int rw)

above function also has 9 arguments not more than what is present
in the block tree. I can trim down the argument list probably by
removing the numa_node as it is set to NUMA_NO_NODE for most of the
drivers.

-ck