lkml.org 
[lkml]   [2016]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] iio: fix sched WARNING "do not call blocking ops when !TASK_RUNNING"
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

\
 
 \ /
  Last update: 2016-08-04 12:21    [W:0.091 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site