Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755721AbcJLQwU (ORCPT ); Wed, 12 Oct 2016 12:52:20 -0400 Received: from mail-qk0-f181.google.com ([209.85.220.181]:35295 "EHLO mail-qk0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755651AbcJLQwO (ORCPT ); Wed, 12 Oct 2016 12:52:14 -0400 MIME-Version: 1.0 In-Reply-To: <20161012160309.GA19146@roeck-us.net> References: <1476133442-17757-1-git-send-email-dianders@chromium.org> <20161012160309.GA19146@roeck-us.net> From: Doug Anderson Date: Wed, 12 Oct 2016 09:27:35 -0700 X-Google-Sender-Auth: ZNeS5Wj3vghvJBRtxx05g6yn1Mw Message-ID: Subject: Re: [v2] timers: Fix usleep_range() in the context of wake_up_process() To: Guenter Roeck Cc: Thomas Gleixner , John Stultz , Andreas Mohr , Brian Norris , Tao Huang , Tony Xie , "open list:ARM/Rockchip SoC..." , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1996 Lines: 57 Hi, On Wed, Oct 12, 2016 at 9:03 AM, Guenter Roeck 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