Re: [PATCH 02/15] block: don't try to poll multi-bio I/Os in __blkdev_direct_IO

Kanchan Joshi
2021-05-16T18:02:33Z

On Wed, May 12, 2021 at 6:50 PM Christoph Hellwig <hch@lst.de> wrote:
>
> If an iocb is split into multiple bios we can't poll for both. So don't
> bother to even try to poll in that case.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/block_dev.c | 37 ++++++++++++++-----------------------
> 1 file changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index b8abccd03e5d..0080a3b710b4 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -375,7 +375,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
> struct blk_plug plug;
> struct blkdev_dio *dio;
> struct bio *bio;
> - bool is_poll = (iocb->ki_flags & IOCB_HIPRI) != 0;
> + bool is_poll = (iocb->ki_flags & IOCB_HIPRI), do_poll = false;
> bool is_read = (iov_iter_rw(iter) == READ), is_sync;
> loff_t pos = iocb->ki_pos;
> blk_qc_t qc = BLK_QC_T_NONE;
> @@ -437,22 +437,9 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
> pos += bio->bi_iter.bi_size;
>
> nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS);
> - if (!nr_pages) {
> - bool polled = false;
> -
> - if (iocb->ki_flags & IOCB_HIPRI) {
> - bio_set_polled(bio, iocb);
> - polled = true;
> - }
> -
> - qc = submit_bio(bio);
> -
> - if (polled)
> - WRITE_ONCE(iocb->ki_cookie, qc);
> - break;
> - }
> -
> - if (!dio->multi_bio) {
> + if (dio->multi_bio) {
> + atomic_inc(&dio->ref);
> + } else if (nr_pages) {
> /*
> * AIO needs an extra reference to ensure the dio
> * structure which is embedded into the first bio
> @@ -462,11 +449,16 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
> bio_get(bio);
> dio->multi_bio = true;
> atomic_set(&dio->ref, 2);
> - } else {
> - atomic_inc(&dio->ref);
> + } else if (is_poll) {
> + bio_set_polled(bio, iocb);
> + do_poll = true;
> + }
> + qc = submit_bio(bio);
> + if (!nr_pages) {
> + if (do_poll)
> + WRITE_ONCE(iocb->ki_cookie, qc);
> + break;
> }
> -
> - submit_bio(bio);
> bio = bio_alloc(GFP_KERNEL, nr_pages);
> }

dio->ref update goes amiss here.
For multi-bio, if the original code sets it as N, this will set it as
N+1, causing endless-wait for the caller.

--
Kanchan