| 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
| ||
|
Message-ID: <6fee2d7a-7fd1-73ee-2911-87a4ed3e8769@opensource.wdc.com>
Date: Wed, 5 Oct 2022 14:11:11 +0900
From: Damien Le Moal <damien.lemoal@...nsource.wdc.com>
To: Chaitanya Kulkarni <kch@...dia.com>, 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
Cc: 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, ulf.hansson@...aro.org,
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,
bvanassche@....org, 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 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.
Using a function with that many arguments will be error prone, and hard to
review... Not a fan.
> +{
> + if (!set)
> + return;
> +
> + set->ops = ops;
> + set->nr_hw_queues = nr_hw_queues;
> + set->queue_depth = queue_depth;
> + set->cmd_size = cmd_size;
> + set->numa_node = numa_node;
> + set->timeout = timeout;
> + set->flags = flags;
> + set->driver_data = driver_data;
> +}
> +
No blank line here.
> +EXPORT_SYMBOL_GPL(blk_mq_init_tag_set);
> +
> /*
> * Alloc a tag set to be associated with one or more request queues.
> * May fail with EINVAL for various error conditions. May adjust the
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 1f154f92f4c2..0b07aab980c4 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1926,13 +1926,9 @@ static int null_init_tag_set(struct nullb *nullb, struct blk_mq_tag_set *set)
> flags |= BLK_MQ_F_BLOCKING;
> }
>
> - set->ops = &null_mq_ops;
> - set->cmd_size = sizeof(struct nullb_cmd);
> - set->flags = flags;
> - set->driver_data = nullb;
> - set->nr_hw_queues = hw_queues;
> - set->queue_depth = queue_depth;
> - set->numa_node = numa_node;
> + blk_mq_init_tag_set(set, &null_mq_ops, hw_queues, queue_depth,
> + sizeof(struct nullb_cmd), numa_node, 0, flags, nullb);
> +
> if (poll_queues) {
> set->nr_hw_queues += poll_queues;
> set->nr_maps = 3;
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index ba18e9bdb799..06087a8e4398 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -708,6 +708,11 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> struct request_queue *q);
> void blk_mq_destroy_queue(struct request_queue *);
>
> +
No need for the extra whiteline.
> +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);
> int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set);
> int blk_mq_alloc_sq_tag_set(struct blk_mq_tag_set *set,
> const struct blk_mq_ops *ops, unsigned int queue_depth,
--
Damien Le Moal
Western Digital Research
Powered by blists - more mailing lists