Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755861AbcJLQyN (ORCPT ); Wed, 12 Oct 2016 12:54:13 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:35408 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755804AbcJLQxx (ORCPT ); Wed, 12 Oct 2016 12:53:53 -0400 Date: Wed, 12 Oct 2016 09:53:51 -0700 From: Guenter Roeck To: Doug Anderson Cc: Thomas Gleixner , John Stultz , Andreas Mohr , Brian Norris , Tao Huang , Tony Xie , "open list:ARM/Rockchip SoC..." , "linux-kernel@vger.kernel.org" Subject: Re: [v2] timers: Fix usleep_range() in the context of wake_up_process() Message-ID: <20161012165351.GA20472@roeck-us.net> References: <1476133442-17757-1-git-send-email-dianders@chromium.org> <20161012160309.GA19146@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2623 Lines: 79 On Wed, Oct 12, 2016 at 09:27:35AM -0700, Doug Anderson wrote: > 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. > 20 ms. > 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); > Correct > 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); > } > Good point. > 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. > Yes, you are correct. > > 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(). > ... which, for those who don't pay attention (like me), is __set_current_state(TASK_UNINTERRUPTIBLE); > 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. > Agreed. > > Thank you for searching through for stuff and for your review, though! > No problem. Thanks for correcting me. Note that I also searched for use of usleep_range() in conjunction with a a task wakeup, but did not find anything. I did find a large number of cases, though, where the explicit assumption is made that the minimum sleep time is well defined. Guenter