* [PATCH] blk: optimization for classic polling [not found] <CGME20180208143655epcas1p444bf34705526b7839a5a135f82761aad@epcas1p4.samsung.com> @ 2083-05-30 4:21 ` Nitesh Shetty 2018-02-08 15:27 ` Keith Busch ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Nitesh Shetty @ 2083-05-30 4:21 UTC (permalink / raw) To: linux-fsdevel, linux-kernel, viro Cc: linux-block, axboe, linux-nvme, nj.shetty, joshi.k This removes the dependency on interrupts to wake up task. Set task state as TASK_RUNNING, if need_resched() returns true, while polling for IO completion. Earlier, polling task used to sleep, relying on interrupt to wake it up. This made some IO take very long when interrupt-coalescing is enabled in NVMe. Reference: http://lists.infradead.org/pipermail/linux-nvme/2018-February/015435.html Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> --- fs/block_dev.c | 16 ++++++++++++---- fs/direct-io.c | 8 ++++++-- fs/iomap.c | 10 +++++++--- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index 4a181fc..a87d8b7 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -236,9 +236,13 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, set_current_state(TASK_UNINTERRUPTIBLE); if (!READ_ONCE(bio.bi_private)) break; - if (!(iocb->ki_flags & IOCB_HIPRI) || - !blk_poll(bdev_get_queue(bdev), qc)) + if (!(iocb->ki_flags & IOCB_HIPRI)) io_schedule(); + else if (!blk_poll(bdev_get_queue(bdev), qc)) { + if (need_resched()) + set_current_state(TASK_RUNNING); + io_schedule(); + } } __set_current_state(TASK_RUNNING); @@ -401,9 +405,13 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) if (!READ_ONCE(dio->waiter)) break; - if (!(iocb->ki_flags & IOCB_HIPRI) || - !blk_poll(bdev_get_queue(bdev), qc)) + if (!(iocb->ki_flags & IOCB_HIPRI)) io_schedule(); + else if (!blk_poll(bdev_get_queue(bdev), qc)) { + if (need_resched()) + set_current_state(TASK_RUNNING); + io_schedule(); + } } __set_current_state(TASK_RUNNING); diff --git a/fs/direct-io.c b/fs/direct-io.c index a0ca9e4..c815ac9 100644 --- a/fs/direct-io.c +++ b/fs/direct-io.c @@ -518,9 +518,13 @@ static struct bio *dio_await_one(struct dio *dio) __set_current_state(TASK_UNINTERRUPTIBLE); dio->waiter = current; spin_unlock_irqrestore(&dio->bio_lock, flags); - if (!(dio->iocb->ki_flags & IOCB_HIPRI) || - !blk_poll(dio->bio_disk->queue, dio->bio_cookie)) + if (!(dio->iocb->ki_flags & IOCB_HIPRI)) io_schedule(); + else if (!blk_poll(dio->bio_disk->queue, dio->bio_cookie)) { + if (need_resched()) + __set_current_state(TASK_RUNNING); + io_schedule(); + } /* wake up sets us TASK_RUNNING */ spin_lock_irqsave(&dio->bio_lock, flags); dio->waiter = NULL; diff --git a/fs/iomap.c b/fs/iomap.c index afd1635..b51569d 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -1072,10 +1072,14 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, break; if (!(iocb->ki_flags & IOCB_HIPRI) || - !dio->submit.last_queue || - !blk_poll(dio->submit.last_queue, - dio->submit.cookie)) + !dio->submit.last_queue) io_schedule(); + else if (!blk_poll(dio->submit.last_queue, + dio->submit.cookie)) { + if (need_resched()) + set_current_state(TASK_RUNNING); + io_schedule(); + } } __set_current_state(TASK_RUNNING); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2083-05-30 4:21 ` [PATCH] blk: optimization for classic polling Nitesh Shetty @ 2018-02-08 15:27 ` Keith Busch 2018-02-08 16:03 ` Sagi Grimberg [not found] ` <CGME20180213082701epcas1p4d92a86f127a0918fb9d62120bd121cb6@epcas1p4.samsung.com> ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Keith Busch @ 2018-02-08 15:27 UTC (permalink / raw) To: Nitesh Shetty Cc: axboe, joshi.k, linux-kernel, linux-nvme, linux-block, viro, linux-fsdevel On Sun, May 30, 2083 at 09:51:06AM +0530, Nitesh Shetty wrote: > This removes the dependency on interrupts to wake up task. Set task > state as TASK_RUNNING, if need_resched() returns true, > while polling for IO completion. > Earlier, polling task used to sleep, relying on interrupt to wake it up. > This made some IO take very long when interrupt-coalescing is enabled in > NVMe. > > Reference: > http://lists.infradead.org/pipermail/linux-nvme/2018-February/015435.html > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> > --- > fs/block_dev.c | 16 ++++++++++++---- > fs/direct-io.c | 8 ++++++-- > fs/iomap.c | 10 +++++++--- > 3 files changed, 25 insertions(+), 9 deletions(-) I think it'd be simpler to have blk_poll set it back to running if need_resched is true rather than repeat this patter across all the callers: --- diff --git a/block/blk-mq.c b/block/blk-mq.c index df93102e2149..40285fe1c8ad 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -3164,6 +3164,7 @@ static bool __blk_mq_poll(struct blk_mq_hw_ctx *hctx, struct request *rq) cpu_relax(); } + set_current_state(TASK_RUNNING); return false; } -- _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2018-02-08 15:27 ` Keith Busch @ 2018-02-08 16:03 ` Sagi Grimberg 0 siblings, 0 replies; 9+ messages in thread From: Sagi Grimberg @ 2018-02-08 16:03 UTC (permalink / raw) To: Keith Busch, Nitesh Shetty Cc: axboe, joshi.k, linux-kernel, linux-nvme, linux-block, viro, linux-fsdevel > I think it'd be simpler to have blk_poll set it back to running if > need_resched is true rather than repeat this patter across all the > callers: > > --- > diff --git a/block/blk-mq.c b/block/blk-mq.c > index df93102e2149..40285fe1c8ad 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -3164,6 +3164,7 @@ static bool __blk_mq_poll(struct blk_mq_hw_ctx *hctx, struct request *rq) > cpu_relax(); > } > > + set_current_state(TASK_RUNNING); > return false; > } > > -- Nice! ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CGME20180213082701epcas1p4d92a86f127a0918fb9d62120bd121cb6@epcas1p4.samsung.com>]
[parent not found: <1518548206-4766-1-git-send-email-nj.shetty@samsung.com>]
* Re: [PATCH v2 RESENT] blk: optimization for classic polling [not found] ` <1518548206-4766-1-git-send-email-nj.shetty@samsung.com> @ 2018-02-13 14:42 ` Jens Axboe 0 siblings, 0 replies; 9+ messages in thread From: Jens Axboe @ 2018-02-13 14:42 UTC (permalink / raw) To: Nitesh Shetty, linux-block, linux-kernel; +Cc: linux-nvme, joshi.k On 2/13/18 11:56 AM, Nitesh Shetty wrote: > This removes the dependency on interrupts to wake up task. Set task > state as TASK_RUNNING, if need_resched() returns true, > while polling for IO completion. > Earlier, polling task used to sleep, relying on interrupt to wake it up. > This made some IO take very long when interrupt-coalescing is enabled in > NVMe. __set_current_state() should suffice here. -- Jens Axboe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2083-05-30 4:21 ` [PATCH] blk: optimization for classic polling Nitesh Shetty 2018-02-08 15:27 ` Keith Busch [not found] ` <CGME20180213082701epcas1p4d92a86f127a0918fb9d62120bd121cb6@epcas1p4.samsung.com> @ 2018-02-20 13:21 ` Peter Zijlstra 2018-02-20 16:27 ` Keith Busch 2018-02-20 22:37 ` Jens Axboe 2018-10-10 18:52 ` Florian Fainelli 3 siblings, 2 replies; 9+ messages in thread From: Peter Zijlstra @ 2018-02-20 13:21 UTC (permalink / raw) To: Nitesh Shetty Cc: linux-fsdevel, linux-kernel, viro, linux-block, axboe, linux-nvme, joshi.k On Sun, May 30, 2083 at 09:51:06AM +0530, Nitesh Shetty wrote: > This removes the dependency on interrupts to wake up task. Set task > state as TASK_RUNNING, if need_resched() returns true, > while polling for IO completion. > Earlier, polling task used to sleep, relying on interrupt to wake it up. > This made some IO take very long when interrupt-coalescing is enabled in > NVMe. This is a horrible Changelog.. it does not in fact explain why the patch works or is correct. Also, set_current_state(TASK_RUNNING) is dodgy (similarly in __blk_mq_poll), why do you need that memory barrier? > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> > --- > fs/block_dev.c | 16 ++++++++++++---- > fs/direct-io.c | 8 ++++++-- > fs/iomap.c | 10 +++++++--- > 3 files changed, 25 insertions(+), 9 deletions(-) > > diff --git a/fs/block_dev.c b/fs/block_dev.c > index 4a181fc..a87d8b7 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -236,9 +236,13 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, > set_current_state(TASK_UNINTERRUPTIBLE); > if (!READ_ONCE(bio.bi_private)) > break; > - if (!(iocb->ki_flags & IOCB_HIPRI) || > - !blk_poll(bdev_get_queue(bdev), qc)) > + if (!(iocb->ki_flags & IOCB_HIPRI)) > io_schedule(); > + else if (!blk_poll(bdev_get_queue(bdev), qc)) { > + if (need_resched()) > + set_current_state(TASK_RUNNING); > + io_schedule(); > + } > } > __set_current_state(TASK_RUNNING); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2018-02-20 13:21 ` [PATCH] " Peter Zijlstra @ 2018-02-20 16:27 ` Keith Busch 2018-02-20 22:37 ` Jens Axboe 1 sibling, 0 replies; 9+ messages in thread From: Keith Busch @ 2018-02-20 16:27 UTC (permalink / raw) To: Peter Zijlstra Cc: Nitesh Shetty, linux-fsdevel, linux-kernel, viro, linux-block, axboe, linux-nvme, joshi.k On Tue, Feb 20, 2018 at 02:21:37PM +0100, Peter Zijlstra wrote: > Also, set_current_state(TASK_RUNNING) is dodgy (similarly in > __blk_mq_poll), why do you need that memory barrier? You're right. The subsequent revision that was committed removed the barrier. The commit is here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=67b4110f8c8d16e588d7730db8e8b01b32c1bd8b I hope the code at least looks more reasonable. The changelog isn't much different, though. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2018-02-20 13:21 ` [PATCH] " Peter Zijlstra 2018-02-20 16:27 ` Keith Busch @ 2018-02-20 22:37 ` Jens Axboe 2018-02-21 8:32 ` Peter Zijlstra 1 sibling, 1 reply; 9+ messages in thread From: Jens Axboe @ 2018-02-20 22:37 UTC (permalink / raw) To: Peter Zijlstra, Nitesh Shetty Cc: linux-fsdevel, linux-kernel, viro, linux-block, linux-nvme, joshi.k On 2/20/18 3:21 AM, Peter Zijlstra wrote: > On Sun, May 30, 2083 at 09:51:06AM +0530, Nitesh Shetty wrote: >> This removes the dependency on interrupts to wake up task. Set task >> state as TASK_RUNNING, if need_resched() returns true, >> while polling for IO completion. >> Earlier, polling task used to sleep, relying on interrupt to wake it up. >> This made some IO take very long when interrupt-coalescing is enabled in >> NVMe. > > This is a horrible Changelog.. it does not in fact explain why the patch > works or is correct. Yeah, that should have been improved. > Also, set_current_state(TASK_RUNNING) is dodgy (similarly in > __blk_mq_poll), why do you need that memory barrier? I pointed that out in the review, and v2 fixed it. v2 is the one that got merged. -- Jens Axboe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2018-02-20 22:37 ` Jens Axboe @ 2018-02-21 8:32 ` Peter Zijlstra 0 siblings, 0 replies; 9+ messages in thread From: Peter Zijlstra @ 2018-02-21 8:32 UTC (permalink / raw) To: Jens Axboe Cc: Nitesh Shetty, linux-fsdevel, linux-kernel, viro, linux-block, linux-nvme, joshi.k On Tue, Feb 20, 2018 at 12:37:07PM -1000, Jens Axboe wrote: > On 2/20/18 3:21 AM, Peter Zijlstra wrote: > > On Sun, May 30, 2083 at 09:51:06AM +0530, Nitesh Shetty wrote: > >> This removes the dependency on interrupts to wake up task. Set task > >> state as TASK_RUNNING, if need_resched() returns true, > >> while polling for IO completion. > >> Earlier, polling task used to sleep, relying on interrupt to wake it up. > >> This made some IO take very long when interrupt-coalescing is enabled in > >> NVMe. > > > > This is a horrible Changelog.. it does not in fact explain why the patch > > works or is correct. > > Yeah, that should have been improved. Being ever more forgetful (I blame the kids) I find Changelogs more and more important, ymmv ;-) > > Also, set_current_state(TASK_RUNNING) is dodgy (similarly in > > __blk_mq_poll), why do you need that memory barrier? > > I pointed that out in the review, and v2 fixed it. v2 is the > one that got merged. Right missed that. In fact, possibly the only reason I saw this is that Nitesh had this computer configured wrong and the email is from the future and thus the very first entry in my lkml folder. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] blk: optimization for classic polling 2083-05-30 4:21 ` [PATCH] blk: optimization for classic polling Nitesh Shetty ` (2 preceding siblings ...) 2018-02-20 13:21 ` [PATCH] " Peter Zijlstra @ 2018-10-10 18:52 ` Florian Fainelli 3 siblings, 0 replies; 9+ messages in thread From: Florian Fainelli @ 2018-10-10 18:52 UTC (permalink / raw) To: Nitesh Shetty, linux-fsdevel, linux-kernel, viro Cc: linux-block, axboe, linux-nvme, joshi.k On 05/29/2083 08:21 PM, Nitesh Shetty wrote: > This removes the dependency on interrupts to wake up task. Set task > state as TASK_RUNNING, if need_resched() returns true, > while polling for IO completion. > Earlier, polling task used to sleep, relying on interrupt to wake it up. > This made some IO take very long when interrupt-coalescing is enabled in > NVMe. > > Reference: > http://lists.infradead.org/pipermail/linux-nvme/2018-February/015435.html > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> There is something seriously wrong with the date on your computer, this patch is dated from May 29th 2083,... I would not be surprised if it messed up people's email client and you did not get any response because of that... -- Florian ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-11 2:16 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20180208143655epcas1p444bf34705526b7839a5a135f82761aad@epcas1p4.samsung.com> 2083-05-30 4:21 ` [PATCH] blk: optimization for classic polling Nitesh Shetty 2018-02-08 15:27 ` Keith Busch 2018-02-08 16:03 ` Sagi Grimberg [not found] ` <CGME20180213082701epcas1p4d92a86f127a0918fb9d62120bd121cb6@epcas1p4.samsung.com> [not found] ` <1518548206-4766-1-git-send-email-nj.shetty@samsung.com> 2018-02-13 14:42 ` [PATCH v2 RESENT] " Jens Axboe 2018-02-20 13:21 ` [PATCH] " Peter Zijlstra 2018-02-20 16:27 ` Keith Busch 2018-02-20 22:37 ` Jens Axboe 2018-02-21 8:32 ` Peter Zijlstra 2018-10-10 18:52 ` Florian Fainelli
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).