* Re: [RFC PATCH] blk-core: remove blk_put_request() [not found] <20210222211115.30416-1-chaitanya.kulkarni@wdc.com> @ 2021-02-24 11:53 ` Stefan Hajnoczi 2021-02-24 16:48 ` Jens Axboe 0 siblings, 1 reply; 3+ messages in thread From: Stefan Hajnoczi @ 2021-02-24 11:53 UTC (permalink / raw) To: Chaitanya Kulkarni Cc: snitzer, mst, virtualization, bfields, linux-ide, dm-devel, target-devel, alim.akhtar, agk, beanhuo, stanley.chu, linux-scsi, cang, tim, dgilbert, vbadigan, richard.peng, jejb, linux-block, avri.altman, bp, jaegeuk, Kai.Makisara, axboe, linux-nfs, martin.petersen, baolin.wang, linux-mmc, linux-kernel, fujita.tomonori, chuck.lever, zliua, pbonzini, davem, asutoshd [-- Attachment #1.1: Type: text/plain, Size: 2511 bytes --] On Mon, Feb 22, 2021 at 01:11:15PM -0800, Chaitanya Kulkarni wrote: > The function blk_put_request() is just a wrapper to > blk_mq_free_request(), remove the unnecessary wrapper. > > Any feedback is welcome on this RFC. > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > --- > block/blk-core.c | 6 ------ > block/blk-merge.c | 2 +- > block/bsg-lib.c | 4 ++-- > block/bsg.c | 4 ++-- > block/scsi_ioctl.c | 6 +++--- > drivers/block/paride/pd.c | 2 +- > drivers/block/pktcdvd.c | 2 +- > drivers/block/virtio_blk.c | 2 +- > drivers/cdrom/cdrom.c | 4 ++-- > drivers/ide/ide-atapi.c | 2 +- > drivers/ide/ide-cd.c | 4 ++-- > drivers/ide/ide-cd_ioctl.c | 2 +- > drivers/ide/ide-devsets.c | 2 +- > drivers/ide/ide-disk.c | 2 +- > drivers/ide/ide-ioctls.c | 4 ++-- > drivers/ide/ide-park.c | 2 +- > drivers/ide/ide-pm.c | 4 ++-- > drivers/ide/ide-tape.c | 2 +- > drivers/ide/ide-taskfile.c | 2 +- > drivers/md/dm-mpath.c | 2 +- > drivers/mmc/core/block.c | 10 +++++----- > drivers/scsi/scsi_error.c | 2 +- > drivers/scsi/scsi_lib.c | 2 +- > drivers/scsi/sg.c | 6 +++--- > drivers/scsi/st.c | 4 ++-- > drivers/scsi/ufs/ufshcd.c | 6 +++--- > drivers/target/target_core_pscsi.c | 4 ++-- > fs/nfsd/blocklayout.c | 4 ++-- > include/linux/blkdev.h | 1 - > 29 files changed, 46 insertions(+), 53 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index fc60ff208497..1754f5e7cc80 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -642,12 +642,6 @@ struct request *blk_get_request(struct request_queue *q, unsigned int op, > } > EXPORT_SYMBOL(blk_get_request); > > -void blk_put_request(struct request *req) > -{ > - blk_mq_free_request(req); > -} > -EXPORT_SYMBOL(blk_put_request); blk_get_request() still exists after this patch. A "get" API usually has a corresponding "put" API. I'm not sure this patch helps the consistency and clarity of the code. If you do go ahead, please update the blk_get_request() doc comment explicitly mentioning that blk_mq_free_request() needs to be called. Stefan [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 183 bytes --] _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] blk-core: remove blk_put_request() 2021-02-24 11:53 ` [RFC PATCH] blk-core: remove blk_put_request() Stefan Hajnoczi @ 2021-02-24 16:48 ` Jens Axboe 2021-02-24 18:55 ` Christoph Hellwig 0 siblings, 1 reply; 3+ messages in thread From: Jens Axboe @ 2021-02-24 16:48 UTC (permalink / raw) To: Stefan Hajnoczi, Chaitanya Kulkarni Cc: snitzer, mst, virtualization, bfields, linux-ide, dm-devel, target-devel, alim.akhtar, agk, beanhuo, stanley.chu, linux-scsi, cang, tim, dgilbert, vbadigan, richard.peng, jejb, linux-block, avri.altman, bp, jaegeuk, Kai.Makisara, linux-nfs, martin.petersen, baolin.wang, linux-mmc, linux-kernel, fujita.tomonori, chuck.lever, zliua, pbonzini, davem, asutoshd On 2/24/21 4:53 AM, Stefan Hajnoczi wrote: > On Mon, Feb 22, 2021 at 01:11:15PM -0800, Chaitanya Kulkarni wrote: >> The function blk_put_request() is just a wrapper to >> blk_mq_free_request(), remove the unnecessary wrapper. >> >> Any feedback is welcome on this RFC. >> >> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> >> --- >> block/blk-core.c | 6 ------ >> block/blk-merge.c | 2 +- >> block/bsg-lib.c | 4 ++-- >> block/bsg.c | 4 ++-- >> block/scsi_ioctl.c | 6 +++--- >> drivers/block/paride/pd.c | 2 +- >> drivers/block/pktcdvd.c | 2 +- >> drivers/block/virtio_blk.c | 2 +- >> drivers/cdrom/cdrom.c | 4 ++-- >> drivers/ide/ide-atapi.c | 2 +- >> drivers/ide/ide-cd.c | 4 ++-- >> drivers/ide/ide-cd_ioctl.c | 2 +- >> drivers/ide/ide-devsets.c | 2 +- >> drivers/ide/ide-disk.c | 2 +- >> drivers/ide/ide-ioctls.c | 4 ++-- >> drivers/ide/ide-park.c | 2 +- >> drivers/ide/ide-pm.c | 4 ++-- >> drivers/ide/ide-tape.c | 2 +- >> drivers/ide/ide-taskfile.c | 2 +- >> drivers/md/dm-mpath.c | 2 +- >> drivers/mmc/core/block.c | 10 +++++----- >> drivers/scsi/scsi_error.c | 2 +- >> drivers/scsi/scsi_lib.c | 2 +- >> drivers/scsi/sg.c | 6 +++--- >> drivers/scsi/st.c | 4 ++-- >> drivers/scsi/ufs/ufshcd.c | 6 +++--- >> drivers/target/target_core_pscsi.c | 4 ++-- >> fs/nfsd/blocklayout.c | 4 ++-- >> include/linux/blkdev.h | 1 - >> 29 files changed, 46 insertions(+), 53 deletions(-) >> >> diff --git a/block/blk-core.c b/block/blk-core.c >> index fc60ff208497..1754f5e7cc80 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -642,12 +642,6 @@ struct request *blk_get_request(struct request_queue *q, unsigned int op, >> } >> EXPORT_SYMBOL(blk_get_request); >> >> -void blk_put_request(struct request *req) >> -{ >> - blk_mq_free_request(req); >> -} >> -EXPORT_SYMBOL(blk_put_request); > > blk_get_request() still exists after this patch. A "get" API usually has > a corresponding "put" API. I'm not sure this patch helps the consistency > and clarity of the code. > > If you do go ahead, please update the blk_get_request() doc comment > explicitly mentioning that blk_mq_free_request() needs to be called. Would make sense to rename blk_get_request() to blk_mq_alloc_request() and then we have API symmetry. The get/put don't make sense when there are no references involved. But it's a lot of churn for very little reward, which is always kind of annoying. Especially for the person that has to carry the patches. -- Jens Axboe _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] blk-core: remove blk_put_request() 2021-02-24 16:48 ` Jens Axboe @ 2021-02-24 18:55 ` Christoph Hellwig 0 siblings, 0 replies; 3+ messages in thread From: Christoph Hellwig @ 2021-02-24 18:55 UTC (permalink / raw) To: Jens Axboe Cc: snitzer, mst, virtualization, bfields, linux-ide, dm-devel, target-devel, alim.akhtar, agk, beanhuo, stanley.chu, linux-scsi, cang, tim, dgilbert, vbadigan, richard.peng, Chaitanya Kulkarni, jejb, linux-block, avri.altman, bp, Stefan Hajnoczi, jaegeuk, Kai.Makisara, linux-nfs, martin.petersen, baolin.wang, linux-mmc, linux-kernel, fujita.tomonori, chuck.lever, zliua, pbonzini, davem, asutoshd On Wed, Feb 24, 2021 at 09:48:21AM -0700, Jens Axboe wrote: > Would make sense to rename blk_get_request() to blk_mq_alloc_request() > and then we have API symmetry. The get/put don't make sense when there > are no references involved. > > But it's a lot of churn for very little reward, which is always kind > of annoying. Especially for the person that has to carry the patches. Let's do the following: - move the initialize_rq_fn call from blk_get_request into blk_mq_alloc_request and make the former a trivial alias for the latter - migrate to the blk_mq_* versions on a per-driver/subsystem basis. The scsi migration depends on the first item above, so it will have to go with that or wait for the next merge window - don't migrate the legacy ide driver, as it is about to be removed and has a huge number of blk_get_request calls _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-24 18:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210222211115.30416-1-chaitanya.kulkarni@wdc.com>
2021-02-24 11:53 ` [RFC PATCH] blk-core: remove blk_put_request() Stefan Hajnoczi
2021-02-24 16:48 ` Jens Axboe
2021-02-24 18:55 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox