From: Satya Tangirala <satyat@google.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org, Salman Qazi <sqazi@google.com>,
Jesse Barnes <jsbarnes@google.com>,
Christoph Hellwig <hch@lst.de>,
Bart Van Assche <bvanassche@acm.org>,
Hannes Reinecke <hare@suse.de>
Subject: Re: [PATCH V2] block: add blk_io_schedule() for avoiding task hung in sync dio
Date: Thu, 14 May 2020 01:28:11 +0000 [thread overview]
Message-ID: <20200514012811.GA84853@google.com> (raw)
In-Reply-To: <20200514011710.GF2073570@T590>
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
>
prev parent reply other threads:[~2020-05-14 1:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
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=20200514012811.GA84853@google.com \
--to=satyat@google.com \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=jsbarnes@google.com \
--cc=linux-block@vger.kernel.org \
--cc=ming.lei@redhat.com \
--cc=sqazi@google.com \
/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).