From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752895AbbEKQt2 (ORCPT ); Mon, 11 May 2015 12:49:28 -0400 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 X-Greylist: delayed 488 seconds by postgrey-1.27 at vger.kernel.org; Mon, 11 May 2015 12:49:26 EDT X-Authority-Analysis: v=2.1 cv=P7Dw/H0u c=1 sm=1 tr=0 a=f/edn6q14DUxYFbwN4BeGQ==:117 a=f/edn6q14DUxYFbwN4BeGQ==:17 a=kj9zAlcOel0A:10 a=h1PgugrvaO0A:10 a=KKAkSRfTAAAA:8 a=VwQbUJbxAAAA:8 a=20KFwNOVAAAA:8 a=z1H5ADGQAAAA:8 a=U4gs-SqVp64p-1N1z1EA:9 a=VIskvqEmYsUUYC-s:21 a=PxCgUCjWZ1AvH62F:21 a=CjuIK1q_8ugA:10 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> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.27; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4wfJP2zG+AferWGbPnIDIUjjW6OzITRckn9xE+4dMMA2qdvEGXjV3Y5ISlIKAGfreOAqJPGWpCW9C21iGo59cQgfHydZmrx6WEB+YaDnChhOhKt66tgqkWNubMLDYCnYj940WtblCAtgcW9EAsIdA7jdxDuptY7tnMMa07Ad+lLQPgIg2KzuyHC594WhPkHrdXLzKLKojUzfB55SeAdYMGVFz/0SAt3hqhbnp38Fal0uig5aWbri4sHSac+kpzEf4+QXRbljPkxGtkhZuiVmXz1eSwO+dy4c8im6MXpLgtTzyiH12wqZ7nktnT52uI+uWAqAVqos2OQKYiI0YvqDC+ySli2jEHCxDw2XKOKcTfKSoqpNo3pIlVYND4eZW0np8aLf5dOPUr/5dE+d0bleQ9+4AhabCGaKtcNPY8hJoaN4Of Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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; > }