Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1456278
| From | Brian Norris <briannorris@chromium.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" |
| Date | 2016-08-04 11:50 +0200 |
| Message-ID | <s2nrQ-76H-7@gated-at.bofh.it> (permalink) |
| References | (1 earlier) <s1If1-589-73@gated-at.bofh.it> <s1LcW-77c-5@gated-at.bofh.it> <s1Lmz-7qe-73@gated-at.bofh.it> <s2mcp-6oY-1@gated-at.bofh.it> <s2mvM-6x9-9@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, Aug 04, 2016 at 10:45:39AM +0200, Lars-Peter Clausen wrote:
> > @@ -132,10 +133,13 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
> > to_wait = min_t(size_t, n / datum_size, rb->watermark);
> >
> > do {
> > - ret = wait_event_interruptible(rb->pollq,
> > - iio_buffer_ready(indio_dev, rb, to_wait, n / datum_size));
> > - if (ret)
> > - return ret;
> > + add_wait_queue(&rb->pollq, &wait);
> > + while (!iio_buffer_ready(indio_dev, rb, to_wait,
> > + n / datum_size)) {
> > + wait_woken(&wait, TASK_INTERRUPTIBLE,
> > + MAX_SCHEDULE_TIMEOUT);
>
> We loose the ability to break out from this loop by sending a signal to the
> task. This needs something like
>
> if (signal_pending(current)) {
> ret = -ERESTARTSYS;
> break;
> }
>
> before the wait_woken()
Sounds good.
> And as a minor improvement I'd also move the
> add_wait_queue()/remove_wait_queue() outside of the outer loop.
Sure.
> And then
> just if (!iio_buffer_ready(...)) continue; rather than having the inner
> loop. This should slightly simplify the flow.
Perhaps I'm not gathering your meaning here, but wouldn't that turn this
into a spin loop, waiting for iio_buffer_ready()? i.e.:
do {
if (!iio_buffer_ready(...))
continue; // we shouldn't just hammer
// iio_buffer_ready(), should we?
wait_woken(...);
...
};
> Just make sure to replace the
> returns in the loop with a break so remove_wait_queue() has a chance to run.
>
>
> > + }
> > + remove_wait_queue(&rb->pollq, &wait);
> >
> > if (!indio_dev->info)
> > return -ENODEV;
> >
>
Brian
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
iio: WARNING at kernel/sched/core.c:7630: do not call blocking ops when !TASK_RUNNING Brian Norris <briannorris@chromium.org> - 2016-08-02 03:30 +0200
Re: iio: WARNING at kernel/sched/core.c:7630: do not call blocking ops when !TASK_RUNNING Lars-Peter Clausen <lars@metafoo.de> - 2016-08-02 15:50 +0200
Re: iio: WARNING at kernel/sched/core.c:7630: do not call blocking ops when !TASK_RUNNING Brian Norris <briannorris@chromium.org> - 2016-08-02 19:00 +0200
Re: iio: WARNING at kernel/sched/core.c:7630: do not call blocking ops when !TASK_RUNNING Lars-Peter Clausen <lars@metafoo.de> - 2016-08-02 19:10 +0200
[PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Brian Norris <briannorris@chromium.org> - 2016-08-04 10:30 +0200
Re: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Lars-Peter Clausen <lars@metafoo.de> - 2016-08-04 10:50 +0200
Re: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Brian Norris <briannorris@chromium.org> - 2016-08-04 11:50 +0200
Re: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Lars-Peter Clausen <lars@metafoo.de> - 2016-08-04 13:20 +0200
Re: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Brian Norris <briannorris@chromium.org> - 2016-08-09 00:30 +0200
Re: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Lars-Peter Clausen <lars@metafoo.de> - 2016-08-09 10:30 +0200
[PATCH v2] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING" Brian Norris <briannorris@chromium.org> - 2016-08-09 02:20 +0200
csiph-web