From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out-04.shaw.ca ([64.59.134.12]:54115 "EHLO smtp-out-04.shaw.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752015AbbEKQt0 (ORCPT ); Mon, 11 May 2015 12:49:26 -0400 Date: Mon, 11 May 2015 11:41:12 -0500 From: Trevor Cordes To: John Stultz Cc: lkml , Nicolas Pitre , Thomas Gleixner , Ingo Molnar , Josh Boyer , One Thousand Gnomes , Subject: Re: [PATCH v3] ktime: Fix ktime_divns to do signed division Message-ID: <20150511114112.04fea52d@pog.tecnopolis.ca> In-Reply-To: <1431118043-23452-1-git-send-email-john.stultz@linaro.org> References: <1431118043-23452-1-git-send-email-john.stultz@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 2015-05-08 John Stultz wrote: > It was noted that the 32bit implementation of ktime_divns() > was doing unsigned division and didn't properly handle > negative values. [...] I have compiled, installed and tested (all weekend) the v3 of the patch against 3.19.5-201.fc21.i686+PAE and it seems to work fine / stable, and fixes my bug. I think it's a done deal! Thanks once again! > Cc: Nicolas Pitre > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Josh Boyer > Cc: One Thousand Gnomes > Cc: Trevor Cordes > Cc: # 3.17+ for regression > Tested-by: Trevor Cordes > Reported-by: Trevor Cordes Tested-by: Trevor Cordes [runtime test i686-PAE] > Signed-off-by: John Stultz > --- > > New in v3: > * Fix casting issue Nicolas pointed out > * Use WARN_ON for 64bit case > > include/linux/ktime.h | 27 +++++++++++++++++++++++---- > kernel/time/hrtimer.c | 11 ++++++++--- > 2 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/include/linux/ktime.h b/include/linux/ktime.h > index 5fc3d10..ab2de1c7 100644 > --- a/include/linux/ktime.h > +++ b/include/linux/ktime.h > @@ -166,19 +166,38 @@ static inline bool ktime_before(const ktime_t > cmp1, const ktime_t cmp2) } > > #if BITS_PER_LONG < 64 > -extern u64 __ktime_divns(const ktime_t kt, s64 div); > -static inline u64 ktime_divns(const ktime_t kt, s64 div) > +extern s64 __ktime_divns(const ktime_t kt, s64 div); > +static inline s64 ktime_divns(const ktime_t kt, s64 div) > { > + /* > + * Negative divisors could cause an inf loop, > + * so bug out here. > + */ > + BUG_ON(div < 0); > if (__builtin_constant_p(div) && !(div >> 32)) { > - u64 ns = kt.tv64; > + s64 ns = kt.tv64; > + int neg = (ns < 0); > + > + if (neg) > + ns = -ns; > do_div(ns, div); > + if (neg) > + ns = -ns; > return ns; > } else { > return __ktime_divns(kt, div); > } > } > #else /* BITS_PER_LONG < 64 */ > -# define ktime_divns(kt, div) (u64)((kt).tv64 / (div)) > +static inline s64 ktime_divns(const ktime_t kt, s64 div) > +{ > + /* > + * 32-bit implementation cannot handle negative divisors, > + * so catch them on 64bit as well. > + */ > + WARN_ON(div < 0); > + return kt.tv64 / div; > +} > #endif > > static inline s64 ktime_to_us(const ktime_t kt) > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > index 76d4bd9..c98ce4d 100644 > --- a/kernel/time/hrtimer.c > +++ b/kernel/time/hrtimer.c > @@ -266,12 +266,15 @@ lock_hrtimer_base(const struct hrtimer *timer, > unsigned long *flags) /* > * Divide a ktime value by a nanosecond value > */ > -u64 __ktime_divns(const ktime_t kt, s64 div) > +s64 __ktime_divns(const ktime_t kt, s64 div) > { > - u64 dclc; > - int sft = 0; > + s64 dclc; > + int neg, sft = 0; > > dclc = ktime_to_ns(kt); > + neg = (dclc < 0); > + if (neg) > + dclc = -dclc; > /* Make sure the divisor is less than 2^32: */ > while (div >> 32) { > sft++; > @@ -279,6 +282,8 @@ u64 __ktime_divns(const ktime_t kt, s64 div) > } > dclc >>= sft; > do_div(dclc, (unsigned long) div); > + if (neg) > + dclc = -dclc; > > return dclc; > }