Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1499825

Re: [v2] timers: Fix usleep_range() in the context of wake_up_process()

Show all headers | View raw

Hi,

On Wed, Oct 12, 2016 at 9:03 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> drivers/iio/accel/kxcjk-1013.c: kxcjk1013_runtime_resume()
> drivers/iio/accel/bmc150-accel-core.c:bmc150_accel_runtime_resume()
> drivers/iio/accel/mma8452.c:mma8452_runtime_resume()
> drivers/iio/accel/mma9551_core.c:mma9551_sleep()

As far as I can tell these drivers will not suffer unduly from my
change.  Worse case they will delay 20us more, which is listed as the
max.

Also note that I assume the reason you flagged these is because they
follow the pattern:

        if (sleep_val < 20000)
                usleep_range(sleep_val, 20000);
        else
                msleep_interruptible(sleep_val/1000);

I will note that usleep_range() is and has always been
uninterruptible, since the implementation says:

void __sched usleep_range(unsigned long min, unsigned long max)
{
       __set_current_state(TASK_UNINTERRUPTIBLE);
       do_usleep_range(min, max);
}

So I'm not at all convinced that we are changing behavior here.  The
"interruptible" vs. "uninterruptible" affects whether signals can
interrupt the sleep, not whether a random wake up of a task can.  What
we really need to know is if they are affected by a wakeup.

> kernel/trace/ring_buffer.c:rb_test()

I assume that the person who wrote this code was confused since they wrote:

  set_current_state(TASK_INTERRUPTIBLE);
  /* Now sleep between a min of 100-300us and a max of 1ms */
  usleep_range(((data->cnt % 3) + 1) * 100, 1000);

That doesn't seem to make sense given the first line of usleep_range().

In any case, again I don't think I am changing behavior.

> A possible solution might be to introduce usleep_range_interruptible()
> and use it there.

This could be a useful function, but I don't think we need it if we
find someone who needs a wakeup to cut short a sleep.  We can just
call one of the schedule functions directly and use a timeout.


Thank you for searching through for stuff and for your review, though!

-Doug

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Douglas Anderson <dianders@chromium.org> - 2016-10-10 23:20 +0200
  Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-11 00:40 +0200
  Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Thomas Gleixner <tglx@linutronix.de> - 2016-10-11 09:20 +0200
    Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-11 18:40 +0200
      Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Mark Brown <broonie@kernel.org> - 2016-10-12 11:30 +0200
    Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Andreas Mohr <andi@lisas.de> - 2016-10-11 21:00 +0200
      Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Thomas Gleixner <tglx@linutronix.de> - 2016-10-12 15:20 +0200
        Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-12 19:40 +0200
    Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Heiko Stuebner <heiko@sntech.de> - 2016-10-11 22:40 +0200
  Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Brian Norris <briannorris@chromium.org> - 2016-10-11 21:00 +0200
  Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Andreas Mohr <andi@lisas.de> - 2016-10-11 21:50 +0200
    Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-11 22:10 +0200
      Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Andreas Mohr <andi@lisas.de> - 2016-10-11 22:50 +0200
  Re: [v2] timers: Fix usleep_range() in the context of  wake_up_process() Guenter Roeck <linux@roeck-us.net> - 2016-10-12 18:10 +0200
    Re: [v2] timers: Fix usleep_range() in the context of  wake_up_process() Guenter Roeck <linux@roeck-us.net> - 2016-10-12 19:00 +0200
    Re: [v2] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-12 19:00 +0200
  Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Daniel Kurtz <djkurtz@chromium.org> - 2016-10-18 15:50 +0200
    Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-18 22:30 +0200
      Re: [PATCH v2] timers: Fix usleep_range() in the context of wake_up_process() Daniel Kurtz <djkurtz@chromium.org> - 2016-10-20 11:00 +0200
        Re: [PATCH v2] timers: Fix usleep_range() in the context of  wake_up_process() Thomas Gleixner <tglx@linutronix.de> - 2016-10-20 12:00 +0200

csiph-web