From: Brian Norris <briannorris@chromium.org>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jonathan Cameron <jic23@kernel.org>,
Hartmut Knaack <knaack.h@gmx.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
Guenter Roeck <linux@roeck-us.net>,
Brian Norris <computersforpeace@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>
Subject: [PATCH v2] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING"
Date: Mon, 8 Aug 2016 17:19:38 -0700 [thread overview]
Message-ID: <20160809001937.GA23658@google.com> (raw)
In-Reply-To: <20160804082621.GA11331@localhost>
When using CONFIG_DEBUG_ATOMIC_SLEEP, the scheduler nicely points out
that we're calling sleeping primitives within the wait_event loop, which
means we might clobber the task state:
[ 10.831289] do not call blocking ops when !TASK_RUNNING; state=1 set at [<ffffffc00026b610>]
[ 10.845531] ------------[ cut here ]------------
[ 10.850161] WARNING: at kernel/sched/core.c:7630
...
[ 12.164333] ---[ end trace 45409966a9a76438 ]---
[ 12.168942] Call trace:
[ 12.171391] [<ffffffc00024ed44>] __might_sleep+0x64/0x90
[ 12.176699] [<ffffffc000954774>] mutex_lock_nested+0x50/0x3fc
[ 12.182440] [<ffffffc0007b9424>] iio_kfifo_buf_data_available+0x28/0x4c
[ 12.189043] [<ffffffc0007b76ac>] iio_buffer_ready+0x60/0xe0
[ 12.194608] [<ffffffc0007b7834>] iio_buffer_read_first_n_outer+0x108/0x1a8
[ 12.201474] [<ffffffc000370d48>] __vfs_read+0x58/0x114
[ 12.206606] [<ffffffc000371740>] vfs_read+0x94/0x118
[ 12.211564] [<ffffffc0003720f8>] SyS_read+0x64/0xb4
[ 12.216436] [<ffffffc000203cb4>] el0_svc_naked+0x24/0x28
To avoid this, we should (a la https://lwn.net/Articles/628628/) use the
wait_woken() function, which avoids the nested sleeping while still
handling races between waiting / wake-events.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2:
* Only add/remove to/from the wait queue once per call
* Restore interruptability via explicit signal_pending() call
* Refactor to avoid nested loop
Note that I did not remove the first (semi-redundant) test for
!indio_dev->info, so as to avoid disturbing the error codes used here.
drivers/iio/industrialio-buffer.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 90462fcf5436..49bf9c59f117 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -107,6 +107,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
{
struct iio_dev *indio_dev = filp->private_data;
struct iio_buffer *rb = indio_dev->buffer;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
size_t datum_size;
size_t to_wait;
int ret;
@@ -131,19 +132,29 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
else
to_wait = min_t(size_t, n / datum_size, rb->watermark);
+ add_wait_queue(&rb->pollq, &wait);
do {
- ret = wait_event_interruptible(rb->pollq,
- iio_buffer_ready(indio_dev, rb, to_wait, n / datum_size));
- if (ret)
- return ret;
+ if (!indio_dev->info) {
+ ret = -ENODEV;
+ break;
+ }
- if (!indio_dev->info)
- return -ENODEV;
+ if (!iio_buffer_ready(indio_dev, rb, to_wait, n / datum_size)) {
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
+
+ wait_woken(&wait, TASK_INTERRUPTIBLE,
+ MAX_SCHEDULE_TIMEOUT);
+ continue;
+ }
ret = rb->access->read_first_n(rb, n, buf);
if (ret == 0 && (filp->f_flags & O_NONBLOCK))
ret = -EAGAIN;
} while (ret == 0);
+ remove_wait_queue(&rb->pollq, &wait);
return ret;
}
--
2.8.0.rc3.226.g39d4020
next prev parent reply other threads:[~2016-08-09 0:19 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-02 1:12 iio: WARNING at kernel/sched/core.c:7630: do not call blocking ops when !TASK_RUNNING Brian Norris
2016-08-02 13:06 ` Lars-Peter Clausen
2016-08-02 16:57 ` Brian Norris
2016-08-02 17:04 ` Lars-Peter Clausen
2016-08-04 8:26 ` [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Brian Norris
2016-08-04 8:45 ` Lars-Peter Clausen
2016-08-04 9:41 ` Brian Norris
2016-08-04 10:21 ` Lars-Peter Clausen
2016-08-08 22:23 ` Brian Norris
2016-08-09 8:22 ` Lars-Peter Clausen
2016-08-09 0:19 ` Brian Norris [this message]
2016-08-15 15:54 ` [PATCH v2] " Jonathan Cameron
2016-08-16 15:27 ` Lars-Peter Clausen
2016-08-21 11:21 ` Jonathan Cameron
2016-08-21 12:26 ` Lars-Peter Clausen
2016-08-21 15:23 ` Jonathan Cameron
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=20160809001937.GA23658@google.com \
--to=briannorris@chromium.org \
--cc=computersforpeace@gmail.com \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=pmeerw@pmeerw.net \
/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).