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

Groups > linux.kernel > #1498503

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

Show all headers | View raw

Hi Doug,

On Mon, Oct 10, 2016 at 11:47:57AM -0700, Doug Anderson wrote:
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 32bf6f75a8fe..ab03c7e403a4 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1898,12 +1898,29 @@ EXPORT_SYMBOL(msleep_interruptible);
>  
>  static void __sched do_usleep_range(unsigned long min, unsigned long max)
>  {
> +	ktime_t start, end;
>  	ktime_t kmin;
>  	u64 delta;
> +	unsigned long elapsed = 0;
> +	int ret;
> +
> +	start = ktime_get();
> +	do {
> +		kmin = ktime_set(0, min * NSEC_PER_USEC);

I believe 'min' is unmodified throughout, and therefore 'kmin' is
computed to be the same minimum timeout in each loop. Shouldn't this be
decreasing on each iteration of the loop? (i.e., either your compute
'kmin' differently here, or you recompute 'min' based on the elapsed
time?)

> +		delta = (u64)(max - min) * NSEC_PER_USEC;
> +		ret = schedule_hrtimeout_range(&kmin, delta, HRTIMER_MODE_REL);
> +
> +		/*
> +		 * If schedule_hrtimeout_range() returns 0 then we actually
> +		 * hit the timeout. If not then we need to re-calculate the
> +		 * new timeout ourselves.
> +		 */
> +		if (ret == 0)
> +			break;
>  
> -	kmin = ktime_set(0, min * NSEC_PER_USEC);
> -	delta = (u64)(max - min) * NSEC_PER_USEC;
> -	schedule_hrtimeout_range(&kmin, delta, HRTIMER_MODE_REL);
> +		end = ktime_get();
> +		elapsed = ktime_to_us(ktime_sub(end, start));
> +	} while (elapsed < min);

I think Andreas had similar comments, but it seemed to me like
ktime_before() might be nicer. But (as Andreas did) you might get into
(premature?) micro-optimizations down that path, and I'm certainly not
an expert there.

>  }
>  
>  /**

Brian

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

Thread

[PATCH] timers: Fix usleep_range() in the context of wake_up_process() Douglas Anderson <dianders@chromium.org> - 2016-10-10 21:00 +0200
  Re: [PATCH] timers: Fix usleep_range() in the context of  wake_up_process() Andreas Mohr <andi@lisas.de> - 2016-10-10 22:00 +0200
    Re: [PATCH] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-10 23:20 +0200
  Re: [PATCH] timers: Fix usleep_range() in the context of  wake_up_process() Brian Norris <briannorris@chromium.org> - 2016-10-10 22:10 +0200
    Re: [PATCH] timers: Fix usleep_range() in the context of wake_up_process() Doug Anderson <dianders@chromium.org> - 2016-10-10 22:20 +0200
      Re: [PATCH] timers: Fix usleep_range() in the context of  wake_up_process() Andreas Mohr <andi@lisas.de> - 2016-10-10 22:50 +0200
    Re: [PATCH] timers: Fix usleep_range() in the context of  wake_up_process() Andreas Mohr <andi@lisas.de> - 2016-10-10 22:50 +0200

csiph-web