iomap: fix the logic about poll io in iomap_dio_bio_actor
diff mbox series

Message ID 20191014144313.26313-1-yangerkun@huawei.com
State New
Headers show
Series
  • iomap: fix the logic about poll io in iomap_dio_bio_actor
Related show

Commit Message

yangerkun Oct. 14, 2019, 2:43 p.m. UTC
Just set REQ_HIPRI for the last bio in iomap_dio_bio_actor. Because
multi bio created by this function can goto different cpu since this
process can be preempted by other process. And in iomap_dio_rw we will
just poll for the last bio. Fix it by only set polled for the last bio.

Signed-off-by: yangerkun <yangerkun@huawei.com>
---
 fs/iomap/direct-io.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

Comments

Christoph Hellwig Oct. 15, 2019, 8:05 a.m. UTC | #1
On Mon, Oct 14, 2019 at 10:43:13PM +0800, yangerkun wrote:
> Just set REQ_HIPRI for the last bio in iomap_dio_bio_actor. Because
> multi bio created by this function can goto different cpu since this
> process can be preempted by other process. And in iomap_dio_rw we will
> just poll for the last bio. Fix it by only set polled for the last bio.

I agree that there is a problem with the separate poll queue now.  But
doing partially polled I/O also doesn't seem very useful.  Until we
can find a way to poll for multiple bios from one kiocb I think we need
to limit polling to iocbs with just a single bio.  Can you look into
that?  __blkdev_direct_IO do_blockdev_direct_IO probably have the same
issues.  The former should be just as simple to fix, and for the latter
it might make sense to drop polling support entirely.
yangerkun Oct. 15, 2019, 1:50 p.m. UTC | #2
On 2019/10/15 16:05, Christoph Hellwig wrote:
> On Mon, Oct 14, 2019 at 10:43:13PM +0800, yangerkun wrote:
>> Just set REQ_HIPRI for the last bio in iomap_dio_bio_actor. Because
>> multi bio created by this function can goto different cpu since this
>> process can be preempted by other process. And in iomap_dio_rw we will
>> just poll for the last bio. Fix it by only set polled for the last bio.
> 
> I agree that there is a problem with the separate poll queue now.  But
> doing partially polled I/O also doesn't seem very useful.  Until we
> can find a way to poll for multiple bios from one kiocb I think we need
> to limit polling to iocbs with just a single bio.  Can you look into
> that?  __blkdev_direct_IO do_blockdev_direct_IO probably have the same
> issues.  The former should be just as simple to fix, and for the latter
> it might make sense to drop polling support entirely.
> 
Thanks for your suggestion, i will try to fix it.

Thanks,
Kun.
>

Patch
diff mbox series

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 1fc28c2da279..05dee6e7ca64 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -59,15 +59,16 @@  int iomap_dio_iopoll(struct kiocb *kiocb, bool spin)
 EXPORT_SYMBOL_GPL(iomap_dio_iopoll);
 
 static void iomap_dio_submit_bio(struct iomap_dio *dio, struct iomap *iomap,
-		struct bio *bio)
+		struct bio *bio, bool is_poll)
 {
 	atomic_inc(&dio->ref);
 
-	if (dio->iocb->ki_flags & IOCB_HIPRI)
+	if (is_poll) {
 		bio_set_polled(bio, dio->iocb);
-
-	dio->submit.last_queue = bdev_get_queue(iomap->bdev);
-	dio->submit.cookie = submit_bio(bio);
+		dio->submit.last_queue = bdev_get_queue(iomap->bdev);
+		dio->submit.cookie = submit_bio(bio);
+	} else
+		submit_bio(bio);
 }
 
 static ssize_t iomap_dio_complete(struct iomap_dio *dio)
@@ -191,7 +192,7 @@  iomap_dio_zero(struct iomap_dio *dio, struct iomap *iomap, loff_t pos,
 	get_page(page);
 	__bio_add_page(bio, page, len, 0);
 	bio_set_op_attrs(bio, REQ_OP_WRITE, flags);
-	iomap_dio_submit_bio(dio, iomap, bio);
+	iomap_dio_submit_bio(dio, iomap, bio, false);
 }
 
 static loff_t
@@ -255,6 +256,8 @@  iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
 
 	do {
 		size_t n;
+		bool is_poll = false;
+
 		if (dio->error) {
 			iov_iter_revert(dio->submit.iter, copied);
 			return 0;
@@ -301,7 +304,12 @@  iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
 		copied += n;
 
 		nr_pages = iov_iter_npages(&iter, BIO_MAX_PAGES);
-		iomap_dio_submit_bio(dio, iomap, bio);
+
+		/* Only set poll for the last bio. */
+		if (!nr_pages && dio->iocb->ki_flags & IOCB_HIPRI)
+			is_poll = true;
+
+		iomap_dio_submit_bio(dio, iomap, bio, is_poll);
 	} while (nr_pages);
 
 	/*