* [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
@ 2020-05-03 1:54 Ming Lei
2020-05-03 22:28 ` Bart Van Assche
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Ming Lei @ 2020-05-03 1:54 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, Salman Qazi, Jesse Barnes,
Christoph Hellwig, Bart Van Assche, Hannes Reinecke
Sync dio could be big, or may take long time in discard or in case of
IO failure.
We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
so apply the same trick for prevent task hung from happening in sync dio.
Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
task hung warning.
Cc: Salman Qazi <sqazi@google.com>
Cc: Jesse Barnes <jsbarnes@google.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
V2:
- drop helper of blk_default_io_timeout()
fs/block_dev.c | 4 ++--
fs/direct-io.c | 2 +-
fs/iomap/direct-io.c | 2 +-
include/linux/blkdev.h | 12 ++++++++++++
4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 5eb30a474f6d..3b396f8c967c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -256,7 +256,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
break;
if (!(iocb->ki_flags & IOCB_HIPRI) ||
!blk_poll(bdev_get_queue(bdev), qc, true))
- io_schedule();
+ blk_io_schedule();
}
__set_current_state(TASK_RUNNING);
@@ -450,7 +450,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
if (!(iocb->ki_flags & IOCB_HIPRI) ||
!blk_poll(bdev_get_queue(bdev), qc, true))
- io_schedule();
+ blk_io_schedule();
}
__set_current_state(TASK_RUNNING);
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 00b4d15bb811..6d5370eac2a8 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -500,7 +500,7 @@ static struct bio *dio_await_one(struct dio *dio)
spin_unlock_irqrestore(&dio->bio_lock, flags);
if (!(dio->iocb->ki_flags & IOCB_HIPRI) ||
!blk_poll(dio->bio_disk->queue, dio->bio_cookie, true))
- io_schedule();
+ blk_io_schedule();
/* wake up sets us TASK_RUNNING */
spin_lock_irqsave(&dio->bio_lock, flags);
dio->waiter = NULL;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 20dde5aadcdd..fd3bd06fabb6 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -561,7 +561,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
!dio->submit.last_queue ||
!blk_poll(dio->submit.last_queue,
dio->submit.cookie, true))
- io_schedule();
+ blk_io_schedule();
}
__set_current_state(TASK_RUNNING);
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f00bd4042295..222eb5f32279 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -27,6 +27,7 @@
#include <linux/percpu-refcount.h>
#include <linux/scatterlist.h>
#include <linux/blkzoned.h>
+#include <linux/sched/sysctl.h>
struct module;
struct scsi_ioctl_command;
@@ -1827,4 +1828,15 @@ static inline void blk_wake_io_task(struct task_struct *waiter)
wake_up_process(waiter);
}
+static inline void blk_io_schedule(void)
+{
+ /* Prevent hang_check timer from firing at us during very long I/O */
+ unsigned long timeout = sysctl_hung_task_timeout_secs * HZ / 2;
+
+ if (timeout)
+ io_schedule_timeout(timeout);
+ else
+ io_schedule();
+}
+
#endif
--
2.25.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
2020-05-03 1:54 [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio Ming Lei
@ 2020-05-03 22:28 ` Bart Van Assche
2020-05-12 2:22 ` Ming Lei
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2020-05-03 22:28 UTC (permalink / raw)
To: Ming Lei, Jens Axboe
Cc: linux-block, Salman Qazi, Jesse Barnes, Christoph Hellwig,
Hannes Reinecke
On 2020-05-02 18:54, Ming Lei wrote:
> Sync dio could be big, or may take long time in discard or in case of
> IO failure.
>
> We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
> so apply the same trick for prevent task hung from happening in sync dio.
>
> Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
> task hung warning.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
2020-05-03 1:54 [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio Ming Lei
2020-05-03 22:28 ` Bart Van Assche
@ 2020-05-12 2:22 ` Ming Lei
2020-05-13 2:33 ` Jens Axboe
2020-05-14 1:11 ` Satya Tangirala
3 siblings, 0 replies; 7+ messages in thread
From: Ming Lei @ 2020-05-12 2:22 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Salman Qazi, Jesse Barnes, Christoph Hellwig,
Bart Van Assche, Hannes Reinecke
On Sun, May 03, 2020 at 09:54:22AM +0800, Ming Lei wrote:
> Sync dio could be big, or may take long time in discard or in case of
> IO failure.
>
> We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
> so apply the same trick for prevent task hung from happening in sync dio.
>
> Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
> task hung warning.
>
> Cc: Salman Qazi <sqazi@google.com>
> Cc: Jesse Barnes <jsbarnes@google.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> V2:
> - drop helper of blk_default_io_timeout()
>
>
> fs/block_dev.c | 4 ++--
> fs/direct-io.c | 2 +-
> fs/iomap/direct-io.c | 2 +-
> include/linux/blkdev.h | 12 ++++++++++++
> 4 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 5eb30a474f6d..3b396f8c967c 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -256,7 +256,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
> break;
> if (!(iocb->ki_flags & IOCB_HIPRI) ||
> !blk_poll(bdev_get_queue(bdev), qc, true))
> - io_schedule();
> + blk_io_schedule();
> }
> __set_current_state(TASK_RUNNING);
>
> @@ -450,7 +450,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
>
> if (!(iocb->ki_flags & IOCB_HIPRI) ||
> !blk_poll(bdev_get_queue(bdev), qc, true))
> - io_schedule();
> + blk_io_schedule();
> }
> __set_current_state(TASK_RUNNING);
>
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 00b4d15bb811..6d5370eac2a8 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -500,7 +500,7 @@ static struct bio *dio_await_one(struct dio *dio)
> spin_unlock_irqrestore(&dio->bio_lock, flags);
> if (!(dio->iocb->ki_flags & IOCB_HIPRI) ||
> !blk_poll(dio->bio_disk->queue, dio->bio_cookie, true))
> - io_schedule();
> + blk_io_schedule();
> /* wake up sets us TASK_RUNNING */
> spin_lock_irqsave(&dio->bio_lock, flags);
> dio->waiter = NULL;
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 20dde5aadcdd..fd3bd06fabb6 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -561,7 +561,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> !dio->submit.last_queue ||
> !blk_poll(dio->submit.last_queue,
> dio->submit.cookie, true))
> - io_schedule();
> + blk_io_schedule();
> }
> __set_current_state(TASK_RUNNING);
> }
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index f00bd4042295..222eb5f32279 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -27,6 +27,7 @@
> #include <linux/percpu-refcount.h>
> #include <linux/scatterlist.h>
> #include <linux/blkzoned.h>
> +#include <linux/sched/sysctl.h>
>
> struct module;
> struct scsi_ioctl_command;
> @@ -1827,4 +1828,15 @@ static inline void blk_wake_io_task(struct task_struct *waiter)
> wake_up_process(waiter);
> }
>
> +static inline void blk_io_schedule(void)
> +{
> + /* Prevent hang_check timer from firing at us during very long I/O */
> + unsigned long timeout = sysctl_hung_task_timeout_secs * HZ / 2;
> +
> + if (timeout)
> + io_schedule_timeout(timeout);
> + else
> + io_schedule();
> +}
> +
> #endif
Hi Jens and Guys,
Ping...
thanks,
Ming
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
2020-05-03 1:54 [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio Ming Lei
2020-05-03 22:28 ` Bart Van Assche
2020-05-12 2:22 ` Ming Lei
@ 2020-05-13 2:33 ` Jens Axboe
2020-05-14 1:11 ` Satya Tangirala
3 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2020-05-13 2:33 UTC (permalink / raw)
To: Ming Lei
Cc: linux-block, Salman Qazi, Jesse Barnes, Christoph Hellwig,
Bart Van Assche, Hannes Reinecke
On 5/2/20 7:54 PM, Ming Lei wrote:
> Sync dio could be big, or may take long time in discard or in case of
> IO failure.
>
> We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
> so apply the same trick for prevent task hung from happening in sync dio.
>
> Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
> task hung warning.
Applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
2020-05-03 1:54 [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio Ming Lei
` (2 preceding siblings ...)
2020-05-13 2:33 ` Jens Axboe
@ 2020-05-14 1:11 ` Satya Tangirala
2020-05-14 1:17 ` Ming Lei
3 siblings, 1 reply; 7+ messages in thread
From: Satya Tangirala @ 2020-05-14 1:11 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Salman Qazi, Jesse Barnes,
Christoph Hellwig, Bart Van Assche, Hannes Reinecke
On Sun, May 03, 2020 at 09:54:22AM +0800, Ming Lei wrote:
> Sync dio could be big, or may take long time in discard or in case of
> IO failure.
>
> We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
> so apply the same trick for prevent task hung from happening in sync dio.
>
> Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
> task hung warning.
>
> Cc: Salman Qazi <sqazi@google.com>
> Cc: Jesse Barnes <jsbarnes@google.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> V2:
> - drop helper of blk_default_io_timeout()
>
>
> fs/block_dev.c | 4 ++--
> fs/direct-io.c | 2 +-
> fs/iomap/direct-io.c | 2 +-
> include/linux/blkdev.h | 12 ++++++++++++
> 4 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 5eb30a474f6d..3b396f8c967c 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -256,7 +256,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
> break;
> if (!(iocb->ki_flags & IOCB_HIPRI) ||
> !blk_poll(bdev_get_queue(bdev), qc, true))
> - io_schedule();
> + blk_io_schedule();
> }
> __set_current_state(TASK_RUNNING);
>
> @@ -450,7 +450,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
>
> if (!(iocb->ki_flags & IOCB_HIPRI) ||
> !blk_poll(bdev_get_queue(bdev), qc, true))
> - io_schedule();
> + blk_io_schedule();
> }
> __set_current_state(TASK_RUNNING);
>
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 00b4d15bb811..6d5370eac2a8 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -500,7 +500,7 @@ static struct bio *dio_await_one(struct dio *dio)
> spin_unlock_irqrestore(&dio->bio_lock, flags);
> if (!(dio->iocb->ki_flags & IOCB_HIPRI) ||
> !blk_poll(dio->bio_disk->queue, dio->bio_cookie, true))
> - io_schedule();
> + blk_io_schedule();
> /* wake up sets us TASK_RUNNING */
> spin_lock_irqsave(&dio->bio_lock, flags);
> dio->waiter = NULL;
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 20dde5aadcdd..fd3bd06fabb6 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -561,7 +561,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> !dio->submit.last_queue ||
> !blk_poll(dio->submit.last_queue,
> dio->submit.cookie, true))
> - io_schedule();
> + blk_io_schedule();
> }
> __set_current_state(TASK_RUNNING);
> }
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index f00bd4042295..222eb5f32279 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -27,6 +27,7 @@
> #include <linux/percpu-refcount.h>
> #include <linux/scatterlist.h>
> #include <linux/blkzoned.h>
> +#include <linux/sched/sysctl.h>
>
> struct module;
> struct scsi_ioctl_command;
> @@ -1827,4 +1828,15 @@ static inline void blk_wake_io_task(struct task_struct *waiter)
> wake_up_process(waiter);
> }
>
> +static inline void blk_io_schedule(void)
> +{
> + /* Prevent hang_check timer from firing at us during very long I/O */
> + unsigned long timeout = sysctl_hung_task_timeout_secs * HZ / 2;
> +
> + if (timeout)
> + io_schedule_timeout(timeout);
> + else
> + io_schedule();
> +}
> +
> #endif
This won't compile without CONFIG_BLOCK, since this patch includes
linux/sched/sysctl.h only inside the #ifdef CONFIG_BLOCK, while blk_io_schedule
is declared outside the #ifdef CONFIG_BLOCK, but references
sysctl_hung_task_timeout_secs defined in linux/sched/sysctl.h. Afaict, the
function isn't needed without CONFIG_BLOCK, so maybe we should move the
function into the #ifdef CONFIG_BLOCK too?
> --
> 2.25.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
2020-05-14 1:11 ` Satya Tangirala
@ 2020-05-14 1:17 ` Ming Lei
2020-05-14 1:28 ` Satya Tangirala
0 siblings, 1 reply; 7+ messages in thread
From: Ming Lei @ 2020-05-14 1:17 UTC (permalink / raw)
To: Satya Tangirala
Cc: Jens Axboe, linux-block, Salman Qazi, Jesse Barnes,
Christoph Hellwig, Bart Van Assche, Hannes Reinecke
On Thu, May 14, 2020 at 01:11:10AM +0000, Satya Tangirala wrote:
> On Sun, May 03, 2020 at 09:54:22AM +0800, Ming Lei wrote:
> > Sync dio could be big, or may take long time in discard or in case of
> > IO failure.
> >
> > We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
> > so apply the same trick for prevent task hung from happening in sync dio.
> >
> > Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
> > task hung warning.
> >
> > Cc: Salman Qazi <sqazi@google.com>
> > Cc: Jesse Barnes <jsbarnes@google.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Bart Van Assche <bvanassche@acm.org>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > V2:
> > - drop helper of blk_default_io_timeout()
> >
> >
> > fs/block_dev.c | 4 ++--
> > fs/direct-io.c | 2 +-
> > fs/iomap/direct-io.c | 2 +-
> > include/linux/blkdev.h | 12 ++++++++++++
> > 4 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > index 5eb30a474f6d..3b396f8c967c 100644
> > --- a/fs/block_dev.c
> > +++ b/fs/block_dev.c
> > @@ -256,7 +256,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
> > break;
> > if (!(iocb->ki_flags & IOCB_HIPRI) ||
> > !blk_poll(bdev_get_queue(bdev), qc, true))
> > - io_schedule();
> > + blk_io_schedule();
> > }
> > __set_current_state(TASK_RUNNING);
> >
> > @@ -450,7 +450,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
> >
> > if (!(iocb->ki_flags & IOCB_HIPRI) ||
> > !blk_poll(bdev_get_queue(bdev), qc, true))
> > - io_schedule();
> > + blk_io_schedule();
> > }
> > __set_current_state(TASK_RUNNING);
> >
> > diff --git a/fs/direct-io.c b/fs/direct-io.c
> > index 00b4d15bb811..6d5370eac2a8 100644
> > --- a/fs/direct-io.c
> > +++ b/fs/direct-io.c
> > @@ -500,7 +500,7 @@ static struct bio *dio_await_one(struct dio *dio)
> > spin_unlock_irqrestore(&dio->bio_lock, flags);
> > if (!(dio->iocb->ki_flags & IOCB_HIPRI) ||
> > !blk_poll(dio->bio_disk->queue, dio->bio_cookie, true))
> > - io_schedule();
> > + blk_io_schedule();
> > /* wake up sets us TASK_RUNNING */
> > spin_lock_irqsave(&dio->bio_lock, flags);
> > dio->waiter = NULL;
> > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> > index 20dde5aadcdd..fd3bd06fabb6 100644
> > --- a/fs/iomap/direct-io.c
> > +++ b/fs/iomap/direct-io.c
> > @@ -561,7 +561,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> > !dio->submit.last_queue ||
> > !blk_poll(dio->submit.last_queue,
> > dio->submit.cookie, true))
> > - io_schedule();
> > + blk_io_schedule();
> > }
> > __set_current_state(TASK_RUNNING);
> > }
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index f00bd4042295..222eb5f32279 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -27,6 +27,7 @@
> > #include <linux/percpu-refcount.h>
> > #include <linux/scatterlist.h>
> > #include <linux/blkzoned.h>
> > +#include <linux/sched/sysctl.h>
> >
> > struct module;
> > struct scsi_ioctl_command;
> > @@ -1827,4 +1828,15 @@ static inline void blk_wake_io_task(struct task_struct *waiter)
> > wake_up_process(waiter);
> > }
> >
> > +static inline void blk_io_schedule(void)
> > +{
> > + /* Prevent hang_check timer from firing at us during very long I/O */
> > + unsigned long timeout = sysctl_hung_task_timeout_secs * HZ / 2;
> > +
> > + if (timeout)
> > + io_schedule_timeout(timeout);
> > + else
> > + io_schedule();
> > +}
> > +
> > #endif
> This won't compile without CONFIG_BLOCK, since this patch includes
Thanks for the report.
> linux/sched/sysctl.h only inside the #ifdef CONFIG_BLOCK, while blk_io_schedule
> is declared outside the #ifdef CONFIG_BLOCK, but references
> sysctl_hung_task_timeout_secs defined in linux/sched/sysctl.h. Afaict, the
> function isn't needed without CONFIG_BLOCK, so maybe we should move the
> function into the #ifdef CONFIG_BLOCK too?
Given io_schedule() is always available, I'd suggest to move
"#include <linux/sched/sysctl.h>" out of '#ifdef CONFIG_BLOCK', does the
following patch fixes the issue?
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 5360696d85ff..bf99a723673b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -4,6 +4,7 @@
#include <linux/sched.h>
#include <linux/sched/clock.h>
+#include <linux/sched/sysctl.h>
#ifdef CONFIG_BLOCK
@@ -27,7 +28,6 @@
#include <linux/percpu-refcount.h>
#include <linux/scatterlist.h>
#include <linux/blkzoned.h>
-#include <linux/sched/sysctl.h>
struct module;
struct scsi_ioctl_command;
thanks,
Ming
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
2020-05-14 1:17 ` Ming Lei
@ 2020-05-14 1:28 ` Satya Tangirala
0 siblings, 0 replies; 7+ messages in thread
From: Satya Tangirala @ 2020-05-14 1:28 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Salman Qazi, Jesse Barnes,
Christoph Hellwig, Bart Van Assche, Hannes Reinecke
On Thu, May 14, 2020 at 09:17:10AM +0800, Ming Lei wrote:
> On Thu, May 14, 2020 at 01:11:10AM +0000, Satya Tangirala wrote:
> > On Sun, May 03, 2020 at 09:54:22AM +0800, Ming Lei wrote:
> > > Sync dio could be big, or may take long time in discard or in case of
> > > IO failure.
> > >
> > > We have prevented task hung in submit_bio_wait() and blk_execute_rq(),
> > > so apply the same trick for prevent task hung from happening in sync dio.
> > >
> > > Add helper of blk_io_schedule() and use io_schedule_timeout() to prevent
> > > task hung warning.
> > >
> > > Cc: Salman Qazi <sqazi@google.com>
> > > Cc: Jesse Barnes <jsbarnes@google.com>
> > > Cc: Christoph Hellwig <hch@lst.de>
> > > Cc: Bart Van Assche <bvanassche@acm.org>
> > > Cc: Hannes Reinecke <hare@suse.de>
> > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > ---
> > > V2:
> > > - drop helper of blk_default_io_timeout()
> > >
> > >
> > > fs/block_dev.c | 4 ++--
> > > fs/direct-io.c | 2 +-
> > > fs/iomap/direct-io.c | 2 +-
> > > include/linux/blkdev.h | 12 ++++++++++++
> > > 4 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > > index 5eb30a474f6d..3b396f8c967c 100644
> > > --- a/fs/block_dev.c
> > > +++ b/fs/block_dev.c
> > > @@ -256,7 +256,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
> > > break;
> > > if (!(iocb->ki_flags & IOCB_HIPRI) ||
> > > !blk_poll(bdev_get_queue(bdev), qc, true))
> > > - io_schedule();
> > > + blk_io_schedule();
> > > }
> > > __set_current_state(TASK_RUNNING);
> > >
> > > @@ -450,7 +450,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
> > >
> > > if (!(iocb->ki_flags & IOCB_HIPRI) ||
> > > !blk_poll(bdev_get_queue(bdev), qc, true))
> > > - io_schedule();
> > > + blk_io_schedule();
> > > }
> > > __set_current_state(TASK_RUNNING);
> > >
> > > diff --git a/fs/direct-io.c b/fs/direct-io.c
> > > index 00b4d15bb811..6d5370eac2a8 100644
> > > --- a/fs/direct-io.c
> > > +++ b/fs/direct-io.c
> > > @@ -500,7 +500,7 @@ static struct bio *dio_await_one(struct dio *dio)
> > > spin_unlock_irqrestore(&dio->bio_lock, flags);
> > > if (!(dio->iocb->ki_flags & IOCB_HIPRI) ||
> > > !blk_poll(dio->bio_disk->queue, dio->bio_cookie, true))
> > > - io_schedule();
> > > + blk_io_schedule();
> > > /* wake up sets us TASK_RUNNING */
> > > spin_lock_irqsave(&dio->bio_lock, flags);
> > > dio->waiter = NULL;
> > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> > > index 20dde5aadcdd..fd3bd06fabb6 100644
> > > --- a/fs/iomap/direct-io.c
> > > +++ b/fs/iomap/direct-io.c
> > > @@ -561,7 +561,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> > > !dio->submit.last_queue ||
> > > !blk_poll(dio->submit.last_queue,
> > > dio->submit.cookie, true))
> > > - io_schedule();
> > > + blk_io_schedule();
> > > }
> > > __set_current_state(TASK_RUNNING);
> > > }
> > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > > index f00bd4042295..222eb5f32279 100644
> > > --- a/include/linux/blkdev.h
> > > +++ b/include/linux/blkdev.h
> > > @@ -27,6 +27,7 @@
> > > #include <linux/percpu-refcount.h>
> > > #include <linux/scatterlist.h>
> > > #include <linux/blkzoned.h>
> > > +#include <linux/sched/sysctl.h>
> > >
> > > struct module;
> > > struct scsi_ioctl_command;
> > > @@ -1827,4 +1828,15 @@ static inline void blk_wake_io_task(struct task_struct *waiter)
> > > wake_up_process(waiter);
> > > }
> > >
> > > +static inline void blk_io_schedule(void)
> > > +{
> > > + /* Prevent hang_check timer from firing at us during very long I/O */
> > > + unsigned long timeout = sysctl_hung_task_timeout_secs * HZ / 2;
> > > +
> > > + if (timeout)
> > > + io_schedule_timeout(timeout);
> > > + else
> > > + io_schedule();
> > > +}
> > > +
> > > #endif
> > This won't compile without CONFIG_BLOCK, since this patch includes
>
> Thanks for the report.
>
> > linux/sched/sysctl.h only inside the #ifdef CONFIG_BLOCK, while blk_io_schedule
> > is declared outside the #ifdef CONFIG_BLOCK, but references
> > sysctl_hung_task_timeout_secs defined in linux/sched/sysctl.h. Afaict, the
> > function isn't needed without CONFIG_BLOCK, so maybe we should move the
> > function into the #ifdef CONFIG_BLOCK too?
>
> Given io_schedule() is always available, I'd suggest to move
> "#include <linux/sched/sysctl.h>" out of '#ifdef CONFIG_BLOCK', does the
> following patch fixes the issue?
>
Yeah sure, that works too :).
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 5360696d85ff..bf99a723673b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -4,6 +4,7 @@
>
> #include <linux/sched.h>
> #include <linux/sched/clock.h>
> +#include <linux/sched/sysctl.h>
>
> #ifdef CONFIG_BLOCK
>
> @@ -27,7 +28,6 @@
> #include <linux/percpu-refcount.h>
> #include <linux/scatterlist.h>
> #include <linux/blkzoned.h>
> -#include <linux/sched/sysctl.h>
>
> struct module;
> struct scsi_ioctl_command;
>
> thanks,
> Ming
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-14 1:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-03 1:54 [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio Ming Lei
2020-05-03 22:28 ` Bart Van Assche
2020-05-12 2:22 ` Ming Lei
2020-05-13 2:33 ` Jens Axboe
2020-05-14 1:11 ` Satya Tangirala
2020-05-14 1:17 ` Ming Lei
2020-05-14 1:28 ` Satya Tangirala
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).