From: Damien Le Moal <damien.lemoal@opensource.wdc.com> To: Chaitanya Kulkarni <kch@nvidia.com>, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-nvme@lists.infradead.org, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org Cc: axboe@kernel.dk, efremov@linux.com, josef@toxicpanda.com, idryomov@gmail.com, dongsheng.yang@easystack.cn, haris.iqbal@ionos.com, jinpu.wang@ionos.com, mst@redhat.com, jasowang@redhat.com, pbonzini@redhat.com, stefanha@redhat.com, ohad@wizery.com, andersson@kernel.org, baolin.wang@linux.alibaba.com, ulf.hansson@linaro.org, richard@nod.at, miquel.raynal@bootlin.com, vigneshr@ti.com, marcan@marcan.st, sven@svenpeter.dev, alyssa@rosenzweig.io, kbusch@kernel.org, hch@lst.de, sagi@grimberg.me, sth@linux.ibm.com, hoeppner@linux.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, svens@linux.ibm.com, jejb@linux.ibm.com, martin.petersen@oracle.com, hare@suse.de, bhelgaas@google.com, john.garry@huawei.com, mcgrof@kernel.org, christophe.jaillet@wanadoo.fr, vaibhavgupta40@gmail.com, wsa+renesas@sang-engineering.com, johannes.thumshirn@wdc.com, bvanassche@acm.org, ming.lei@redhat.com, shinichiro.kawasaki@wdc.com, vincent.fu@samsung.com, christoph.boehmwalder@linbit.com, joel@jms.id.au, vincent.whitchurch@axis.com, nbd@other.debian.org, ceph-devel@vger.kernel.org, virtualization@lists.linux-foundation.org, asahi@lists.linux.dev Subject: Re: [RFC PATCH 01/21] block: add and use init tagset helper Date: Wed, 5 Oct 2022 14:11:11 +0900 [thread overview] Message-ID: <6fee2d7a-7fd1-73ee-2911-87a4ed3e8769@opensource.wdc.com> (raw) In-Reply-To: <20221005032257.80681-2-kch@nvidia.com> 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@nvidia.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
WARNING: multiple messages have this Message-ID (diff)
From: Damien Le Moal <damien.lemoal@opensource.wdc.com> To: Chaitanya Kulkarni <kch@nvidia.com>, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-nvme@lists.infradead.org, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org Cc: axboe@kernel.dk, efremov@linux.com, josef@toxicpanda.com, idryomov@gmail.com, dongsheng.yang@easystack.cn, haris.iqbal@ionos.com, jinpu.wang@ionos.com, mst@redhat.com, jasowang@redhat.com, pbonzini@redhat.com, stefanha@redhat.com, ohad@wizery.com, andersson@kernel.org, baolin.wang@linux.alibaba.com, ulf.hansson@linaro.org, richard@nod.at, miquel.raynal@bootlin.com, vigneshr@ti.com, marcan@marcan.st, sven@svenpeter.dev, alyssa@rosenzweig.io, kbusch@kernel.org, hch@lst.de, sagi@grimberg.me, sth@linux.ibm.com, hoeppner@linux.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, svens@linux.ibm.com, jejb@linux.ibm.com, martin.petersen@oracle.com, hare@suse.de, bhelgaas@google.com, john.garry@huawei.com, mcgrof@kernel.org, christophe.jaillet@wanadoo.fr, vaibhavgupta40@gmail.com, wsa+renesas@sang-engineering.com, johannes.thumshirn@wdc.com, bvanassche@acm.org, ming.lei@redhat.com, shinichiro.kawasaki@wdc.com, vincent.fu@samsung.com, christoph.boehmwalder@linbit.com, joel@jms.id.au, vincent.whitchurch@axis.com, nbd@other.debian.org, ceph-devel@vger.kernel.org, virtualization@lists.linux-foundation.org, asahi@lists.linux.dev Subject: Re: [RFC PATCH 01/21] block: add and use init tagset helper Date: Wed, 5 Oct 2022 14:11:11 +0900 [thread overview] Message-ID: <6fee2d7a-7fd1-73ee-2911-87a4ed3e8769@opensource.wdc.com> (raw) In-Reply-To: <20221005032257.80681-2-kch@nvidia.com> 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@nvidia.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 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2022-10-05 5:12 UTC|newest] Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-10-05 3:22 [RFC PATCH 00/21] block: add and use init tagset helper Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 01/21] " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 5:11 ` Damien Le Moal [this message] 2022-10-05 5:11 ` Damien Le Moal 2022-10-05 5:37 ` Chaitanya Kulkarni 2022-10-05 9:47 ` Ulf Hansson 2022-10-05 9:47 ` Ulf Hansson 2022-10-05 9:47 ` Ulf Hansson 2022-10-05 16:54 ` Bart Van Assche 2022-10-05 16:54 ` Bart Van Assche 2022-10-05 16:54 ` Bart Van Assche 2022-10-05 17:22 ` Chaitanya Kulkarni 2022-10-06 18:13 ` Chaitanya Kulkarni 2022-10-06 18:40 ` Bart Van Assche 2022-10-07 19:40 ` Jens Axboe 2022-10-05 3:22 ` [RFC PATCH 02/21] loop: use lib tagset init helper Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 03/21] nbd: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 04/21] rnbd: " Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 05/21] bsg-lib: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 06/21] rnbd-clt: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 07/21] virtio-blk: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` [RFC PATCH 08/21] scsi: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` [RFC PATCH 09/21] block: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 10/21] amiflop: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 11/21] floppy: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 12/21] mtip32xx: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 13/21] z3ram: " Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 14/21] scm_blk: " Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 15/21] ubi: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 16/21] mmc: core: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 17/21] dasd: " Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 18/21] nvme-core: use lib tagset init helper for I/O q Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 19/21] nvme-core: use lib tagset init helper for adminq Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` [RFC PATCH 20/21] nvme-apple: use lib tagset init helper Chaitanya Kulkarni via Virtualization 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 11:01 ` kernel test robot 2022-10-05 3:22 ` [RFC PATCH 21/21] nvme-pci: " Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-05 3:22 ` Chaitanya Kulkarni 2022-10-07 18:26 ` [RFC PATCH 00/21] block: add and use init tagset helper Luis Chamberlain 2022-10-07 18:26 ` Luis Chamberlain 2022-10-07 18:26 ` Luis Chamberlain 2022-10-10 7:55 ` Christoph Hellwig 2022-10-10 7:55 ` Christoph Hellwig 2022-10-10 7:55 ` Christoph Hellwig 2022-10-10 17:06 ` Chaitanya Kulkarni
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=6fee2d7a-7fd1-73ee-2911-87a4ed3e8769@opensource.wdc.com \ --to=damien.lemoal@opensource.wdc.com \ --cc=agordeev@linux.ibm.com \ --cc=alyssa@rosenzweig.io \ --cc=andersson@kernel.org \ --cc=asahi@lists.linux.dev \ --cc=axboe@kernel.dk \ --cc=baolin.wang@linux.alibaba.com \ --cc=bhelgaas@google.com \ --cc=borntraeger@linux.ibm.com \ --cc=bvanassche@acm.org \ --cc=ceph-devel@vger.kernel.org \ --cc=christoph.boehmwalder@linbit.com \ --cc=christophe.jaillet@wanadoo.fr \ --cc=dongsheng.yang@easystack.cn \ --cc=efremov@linux.com \ --cc=gor@linux.ibm.com \ --cc=hare@suse.de \ --cc=haris.iqbal@ionos.com \ --cc=hca@linux.ibm.com \ --cc=hch@lst.de \ --cc=hoeppner@linux.ibm.com \ --cc=idryomov@gmail.com \ --cc=jasowang@redhat.com \ --cc=jejb@linux.ibm.com \ --cc=jinpu.wang@ionos.com \ --cc=joel@jms.id.au \ --cc=johannes.thumshirn@wdc.com \ --cc=john.garry@huawei.com \ --cc=josef@toxicpanda.com \ --cc=kbusch@kernel.org \ --cc=kch@nvidia.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mmc@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=linux-nvme@lists.infradead.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux-remoteproc@vger.kernel.org \ --cc=linux-s390@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=marcan@marcan.st \ --cc=martin.petersen@oracle.com \ --cc=mcgrof@kernel.org \ --cc=ming.lei@redhat.com \ --cc=miquel.raynal@bootlin.com \ --cc=mst@redhat.com \ --cc=nbd@other.debian.org \ --cc=ohad@wizery.com \ --cc=pbonzini@redhat.com \ --cc=richard@nod.at \ --cc=sagi@grimberg.me \ --cc=shinichiro.kawasaki@wdc.com \ --cc=stefanha@redhat.com \ --cc=sth@linux.ibm.com \ --cc=sven@svenpeter.dev \ --cc=svens@linux.ibm.com \ --cc=ulf.hansson@linaro.org \ --cc=vaibhavgupta40@gmail.com \ --cc=vigneshr@ti.com \ --cc=vincent.fu@samsung.com \ --cc=vincent.whitchurch@axis.com \ --cc=virtualization@lists.linux-foundation.org \ --cc=wsa+renesas@sang-engineering.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.