From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>,
Jeffle Xu <jefflexu@linux.alibaba.com>,
Damien Le Moal <Damien.LeMoal@wdc.com>,
linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 12/12] block: switch polling to be bio based
Date: Mon, 26 Apr 2021 23:27:28 +0800 [thread overview]
Message-ID: <YIbb4BFg/j36HlgD@T590> (raw)
In-Reply-To: <20210426134821.2191160-13-hch@lst.de>
On Mon, Apr 26, 2021 at 03:48:21PM +0200, Christoph Hellwig wrote:
> Replace the blk_poll interface that requires the caller to keep a queue
> and cookie from the submissions with polling based on the bio.
>
> Polling for the bio itself leads to a few advantages:
>
> - the cookie construction can made entirely private in blk-mq.c
> - the caller does not need to remember the request_queue and cookie
> separately and thus sidesteps their lifetime issues
> - keeping the device and the cookie inside the bio allows to trivially
> support polling BIOs remapping by stacking drivers
> - a lot of code to propagate the cookie back up the submission path can
> be removed entirely.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/m68k/emu/nfblock.c | 3 +-
> arch/xtensa/platforms/iss/simdisk.c | 3 +-
> block/bio.c | 1 +
> block/blk-core.c | 103 +++++++++++++++++++---------
> block/blk-mq.c | 75 +++++++-------------
> block/blk-mq.h | 2 +
> drivers/block/brd.c | 12 ++--
> drivers/block/drbd/drbd_int.h | 2 +-
> drivers/block/drbd/drbd_req.c | 3 +-
> drivers/block/n64cart.c | 12 ++--
> drivers/block/null_blk/main.c | 3 +-
> drivers/block/pktcdvd.c | 7 +-
> drivers/block/ps3vram.c | 6 +-
> drivers/block/rsxx/dev.c | 7 +-
> drivers/block/umem.c | 4 +-
> drivers/block/zram/zram_drv.c | 10 +--
> drivers/lightnvm/pblk-init.c | 6 +-
> drivers/md/bcache/request.c | 13 ++--
> drivers/md/bcache/request.h | 4 +-
> drivers/md/dm.c | 28 +++-----
> drivers/md/md.c | 10 ++-
> drivers/nvdimm/blk.c | 5 +-
> drivers/nvdimm/btt.c | 5 +-
> drivers/nvdimm/pmem.c | 3 +-
> drivers/nvme/host/core.c | 2 +-
> drivers/nvme/host/multipath.c | 6 +-
> drivers/nvme/host/nvme.h | 2 +-
> drivers/s390/block/dcssblk.c | 7 +-
> drivers/s390/block/xpram.c | 5 +-
> fs/block_dev.c | 25 +++----
> fs/btrfs/inode.c | 8 +--
> fs/ext4/file.c | 2 +-
> fs/gfs2/file.c | 4 +-
> fs/iomap/direct-io.c | 39 ++++-------
> fs/xfs/xfs_file.c | 2 +-
> fs/zonefs/super.c | 2 +-
> include/linux/bio.h | 2 +-
> include/linux/blk-mq.h | 15 +---
> include/linux/blk_types.h | 12 ++--
> include/linux/blkdev.h | 8 ++-
> include/linux/fs.h | 6 +-
> include/linux/iomap.h | 3 +-
> mm/page_io.c | 8 +--
> 43 files changed, 208 insertions(+), 277 deletions(-)
>
> diff --git a/arch/m68k/emu/nfblock.c b/arch/m68k/emu/nfblock.c
> index ba808543161a..dd36808f0d5e 100644
> --- a/arch/m68k/emu/nfblock.c
> +++ b/arch/m68k/emu/nfblock.c
> @@ -59,7 +59,7 @@ struct nfhd_device {
> struct gendisk *disk;
> };
>
> -static blk_qc_t nfhd_submit_bio(struct bio *bio)
> +static void nfhd_submit_bio(struct bio *bio)
> {
> struct nfhd_device *dev = bio->bi_bdev->bd_disk->private_data;
> struct bio_vec bvec;
> @@ -77,7 +77,6 @@ static blk_qc_t nfhd_submit_bio(struct bio *bio)
> sec += len;
> }
> bio_endio(bio);
> - return BLK_QC_T_NONE;
> }
>
> static int nfhd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
> diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
> index fc09be7b1347..182825d639e2 100644
> --- a/arch/xtensa/platforms/iss/simdisk.c
> +++ b/arch/xtensa/platforms/iss/simdisk.c
> @@ -101,7 +101,7 @@ static void simdisk_transfer(struct simdisk *dev, unsigned long sector,
> spin_unlock(&dev->lock);
> }
>
> -static blk_qc_t simdisk_submit_bio(struct bio *bio)
> +static void simdisk_submit_bio(struct bio *bio)
> {
> struct simdisk *dev = bio->bi_bdev->bd_disk->private_data;
> struct bio_vec bvec;
> @@ -119,7 +119,6 @@ static blk_qc_t simdisk_submit_bio(struct bio *bio)
> }
>
> bio_endio(bio);
> - return BLK_QC_T_NONE;
> }
>
> static int simdisk_open(struct block_device *bdev, fmode_t mode)
> diff --git a/block/bio.c b/block/bio.c
> index 7296abe293de..484b6d786857 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -259,6 +259,7 @@ void bio_init(struct bio *bio, struct bio_vec *table,
> memset(bio, 0, sizeof(*bio));
> atomic_set(&bio->__bi_remaining, 1);
> atomic_set(&bio->__bi_cnt, 1);
> + bio->bi_cookie = BLK_QC_T_NONE;
>
> bio->bi_io_vec = table;
> bio->bi_max_vecs = max_vecs;
> diff --git a/block/blk-core.c b/block/blk-core.c
> index adfab5976be0..77fdb00fcad3 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -910,18 +910,18 @@ static noinline_for_stack bool submit_bio_checks(struct bio *bio)
> return false;
> }
>
> -static blk_qc_t __submit_bio(struct bio *bio)
> +static void __submit_bio(struct bio *bio)
> {
> struct gendisk *disk = bio->bi_bdev->bd_disk;
> - blk_qc_t ret = BLK_QC_T_NONE;
>
> if (blk_crypto_bio_prep(&bio)) {
> - if (!disk->fops->submit_bio)
> - return blk_mq_submit_bio(bio);
> - ret = disk->fops->submit_bio(bio);
> + if (!disk->fops->submit_bio) {
> + blk_mq_submit_bio(bio);
> + return;
> + }
> + disk->fops->submit_bio(bio);
> }
> blk_queue_exit(disk->queue);
> - return ret;
> }
>
> /*
> @@ -943,10 +943,9 @@ static blk_qc_t __submit_bio(struct bio *bio)
> * bio_list_on_stack[1] contains bios that were submitted before the current
> * ->submit_bio_bio, but that haven't been processed yet.
> */
> -static blk_qc_t __submit_bio_noacct(struct bio *bio)
> +static void __submit_bio_noacct(struct bio *bio)
> {
> struct bio_list bio_list_on_stack[2];
> - blk_qc_t ret = BLK_QC_T_NONE;
>
> BUG_ON(bio->bi_next);
>
> @@ -966,7 +965,7 @@ static blk_qc_t __submit_bio_noacct(struct bio *bio)
> bio_list_on_stack[1] = bio_list_on_stack[0];
> bio_list_init(&bio_list_on_stack[0]);
>
> - ret = __submit_bio(bio);
> + __submit_bio(bio);
>
> /*
> * Sort new bios into those for a lower level and those for the
> @@ -989,13 +988,11 @@ static blk_qc_t __submit_bio_noacct(struct bio *bio)
> } while ((bio = bio_list_pop(&bio_list_on_stack[0])));
>
> current->bio_list = NULL;
> - return ret;
> }
>
> -static blk_qc_t __submit_bio_noacct_mq(struct bio *bio)
> +static void __submit_bio_noacct_mq(struct bio *bio)
> {
> struct bio_list bio_list[2] = { };
> - blk_qc_t ret = BLK_QC_T_NONE;
>
> current->bio_list = bio_list;
>
> @@ -1007,15 +1004,13 @@ static blk_qc_t __submit_bio_noacct_mq(struct bio *bio)
>
> if (!blk_crypto_bio_prep(&bio)) {
> blk_queue_exit(disk->queue);
> - ret = BLK_QC_T_NONE;
> continue;
> }
>
> - ret = blk_mq_submit_bio(bio);
> + blk_mq_submit_bio(bio);
> } while ((bio = bio_list_pop(&bio_list[0])));
>
> current->bio_list = NULL;
> - return ret;
> }
>
> /**
> @@ -1027,10 +1022,10 @@ static blk_qc_t __submit_bio_noacct_mq(struct bio *bio)
> * systems and other upper level users of the block layer should use
> * submit_bio() instead.
> */
> -blk_qc_t submit_bio_noacct(struct bio *bio)
> +void submit_bio_noacct(struct bio *bio)
> {
> if (!submit_bio_checks(bio))
> - return BLK_QC_T_NONE;
> + return;
>
> /*
> * We only want one ->submit_bio to be active at a time, else stack
> @@ -1038,14 +1033,12 @@ blk_qc_t submit_bio_noacct(struct bio *bio)
> * to collect a list of requests submited by a ->submit_bio method while
> * it is active, and then process them after it returned.
> */
> - if (current->bio_list) {
> + if (current->bio_list)
> bio_list_add(¤t->bio_list[0], bio);
> - return BLK_QC_T_NONE;
> - }
> -
> - if (!bio->bi_bdev->bd_disk->fops->submit_bio)
> - return __submit_bio_noacct_mq(bio);
> - return __submit_bio_noacct(bio);
> + else if (!bio->bi_bdev->bd_disk->fops->submit_bio)
> + __submit_bio_noacct_mq(bio);
> + else
> + __submit_bio_noacct(bio);
> }
> EXPORT_SYMBOL(submit_bio_noacct);
>
> @@ -1062,10 +1055,10 @@ EXPORT_SYMBOL(submit_bio_noacct);
> * in @bio. The bio must NOT be touched by thecaller until ->bi_end_io() has
> * been called.
> */
> -blk_qc_t submit_bio(struct bio *bio)
> +void submit_bio(struct bio *bio)
> {
> if (blkcg_punt_bio_submit(bio))
> - return BLK_QC_T_NONE;
> + return;
>
> /*
> * If it's a regular read/write or a barrier with data attached,
> @@ -1106,19 +1099,67 @@ blk_qc_t submit_bio(struct bio *bio)
> if (unlikely(bio_op(bio) == REQ_OP_READ &&
> bio_flagged(bio, BIO_WORKINGSET))) {
> unsigned long pflags;
> - blk_qc_t ret;
>
> psi_memstall_enter(&pflags);
> - ret = submit_bio_noacct(bio);
> + submit_bio_noacct(bio);
> psi_memstall_leave(&pflags);
> -
> - return ret;
> + return;
> }
>
> - return submit_bio_noacct(bio);
> + submit_bio_noacct(bio);
> }
> EXPORT_SYMBOL(submit_bio);
>
> +/**
> + * bio_poll - poll for BIO completions
> + * @bio: bio to poll for
> + *
> + * Poll for completions on queue associated with the bio. Returns number of
> + * completed entries found. If @spin is true, then bio_poll will continue
> + * looping until at least one completion is found, unless the task is
> + * otherwise marked running (or we need to reschedule).
> + *
> + * Note: the caller must either be the context that submitted @bio, or
> + * be in a RCU critical section to prevent freeing of @bio.
> + */
> +int bio_poll(struct bio *bio, bool spin)
> +{
> + struct request_queue *q = bio->bi_bdev->bd_disk->queue;
> + blk_qc_t cookie = READ_ONCE(bio->bi_cookie);
> +
> + if (cookie == BLK_QC_T_NONE ||
> + !test_bit(QUEUE_FLAG_POLL, &q->queue_flags))
> + return 0;
> +
> + if (current->plug)
> + blk_flush_plug_list(current->plug, false);
> +
> + /* not yet implemented, so this should not happen */
> + if (WARN_ON_ONCE(!queue_is_mq(q)))
> + return 0;
> + return blk_mq_poll(q, cookie, spin);
> +}
> +EXPORT_SYMBOL_GPL(bio_poll);
> +
> +/*
> + * Helper to implements file_operations.iopoll. Requires the bio to be stored
> + * in iocb->private, and cleared before freeing the bio.
> + */
> +int iocb_bio_iopoll(struct kiocb *kiocb, bool spin)
> +{
> + struct bio *bio;
> + int ret = 0;
> +
> + rcu_read_lock();
> + bio = READ_ONCE(kiocb->private);
> + if (bio)
> + ret = bio_poll(bio, spin);
> + rcu_read_unlock();
If bio_poll() sleeps in case of 'spin', rcu_read_lock() can't be used.
And the POLLED bio may be ended by iocb_bio_iopoll() explicitly via
one bio flag.
Thanks,
Ming
next prev parent reply other threads:[~2021-04-26 15:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-26 13:48 switch block layer polling to a bio based model Christoph Hellwig
2021-04-26 13:48 ` [PATCH 01/12] direct-io: remove blk_poll support Christoph Hellwig
2021-04-26 13:48 ` [PATCH 02/12] block: don't try to poll multi-bio I/Os in __blkdev_direct_IO Christoph Hellwig
2021-04-26 13:48 ` [PATCH 03/12] iomap: don't try to poll multi-bio I/Os in __iomap_dio_rw Christoph Hellwig
2021-04-26 13:48 ` [PATCH 04/12] blk-mq: factor out a "classic" poll helper Christoph Hellwig
2021-04-26 13:48 ` [PATCH 05/12] blk-mq: factor out a blk_qc_to_hctx helper Christoph Hellwig
2021-04-26 13:48 ` [PATCH 06/12] blk-mq: refactor hybrid polling Christoph Hellwig
2021-04-26 13:48 ` [PATCH 07/12] blk-mq: remove blk_qc_t_to_tag and blk_qc_t_is_internal Christoph Hellwig
2021-04-26 13:48 ` [PATCH 08/12] blk-mq: remove blk_qc_t_valid Christoph Hellwig
2021-04-26 13:48 ` [PATCH 09/12] block: rename REQ_HIPRI to REQ_POLLED Christoph Hellwig
2021-04-26 13:48 ` [PATCH 10/12] block: RCU free polled bios Christoph Hellwig
2021-04-26 13:48 ` [PATCH 11/12] block: define 'struct bvec_iter' as packed Christoph Hellwig
2021-04-26 13:48 ` [PATCH 12/12] block: switch polling to be bio based Christoph Hellwig
2021-04-26 15:27 ` Ming Lei [this message]
2021-04-26 14:57 ` switch block layer polling to a bio based model Jens Axboe
2021-04-26 15:06 ` Christoph Hellwig
2021-04-26 15:12 ` Jens Axboe
2021-04-26 16:15 ` Christoph Hellwig
2021-04-26 16:48 ` Jens Axboe
2021-04-26 17:05 ` Christoph Hellwig
2021-04-26 17:18 ` Jens Axboe
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=YIbb4BFg/j36HlgD@T590 \
--to=ming.lei@redhat.com \
--cc=Damien.LeMoal@wdc.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=jefflexu@linux.alibaba.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).