lkml.org 
[lkml]   [2016]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [v2] timers: Fix usleep_range() in the context of wake_up_process()
    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

    \
     
     \ /
      Last update: 2016-10-12 18:53    [W:4.043 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site