From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751761AbbEBIcF (ORCPT ); Sat, 2 May 2015 04:32:05 -0400 Received: from smtp-out-01.shaw.ca ([64.59.136.137]:39808 "EHLO smtp-out-01.shaw.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751189AbbEBIcA (ORCPT ); Sat, 2 May 2015 04:32:00 -0400 X-Authority-Analysis: v=2.1 cv=HfbeNHw8 c=1 sm=1 tr=0 a=f/edn6q14DUxYFbwN4BeGQ==:117 a=f/edn6q14DUxYFbwN4BeGQ==:17 a=kj9zAlcOel0A:10 a=h1PgugrvaO0A:10 a=VwQbUJbxAAAA:8 a=KKAkSRfTAAAA:8 a=20KFwNOVAAAA:8 a=z1H5ADGQAAAA:8 a=nWV4HQyl792rcFupqjMA:9 a=hgSgCSDvpIIxfrgt:21 a=sTemREImoE1bbjOl:21 a=CjuIK1q_8ugA:10 Date: Sat, 2 May 2015 03:31:56 -0500 From: Trevor Cordes To: John Stultz Cc: linux-kernel@vger.kernel.org, Nicolas Pitre , Thomas Gleixner , Josh Boyer , One Thousand Gnomes Subject: Re: [PATCH] ktime: Fix ktime_divns to do signed division Message-ID: <20150502033156.45624ff1@pog.tecnopolis.ca> In-Reply-To: <1430520085-24535-1-git-send-email-john.stultz@linaro.org> References: <1430520085-24535-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: MS4wfLYZIX4q4SWFru8aDkZAr9Zt8+hfu+EC+DbaHYWGxAl1/6ifdgYltkPQFusrTqBPxRWaacI6uhSaQ+HZEH4vG0GwgptnzJbvF95qeGbiGd98USfa2Af/7VHv2TbcP7jd4OP+SH2U12rA0m7QDJATqSY0RRDBe6sNyLN8264jf8mImgXnT1ikFrtqrVrbxQApgU8Ja9izGjLDb6LdiZVITOJTQ6gUy3LTpPuJzJBlUktlirGFHELuDJT/up/p2zNuMC6Jsklakp4yicMrEq9rH80o6FGlckWHOLgXQS0Ul9icwswhJemG49sbLXhF3XivVYzt9AWcMmNSa1j+dPC3HDDSESEEJqm2JmdPmSzEgZjC Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015-05-01 John Stultz wrote: > It was noted that the 32bit implementation of ktime_divns > was doing unsgined division adn didn't properly handle > negative values. > > This patch fixes the problem by checking and preserving > the sign bit, and then reapplying it if appropriate > after the division. I worked your new patch into my test system into Fedora 21's latest 3.19.5-200.fc21 (they haven't switched to 4.0 yet). I had to add in Nicolas' 8b618628 commit as well to get this new patch to go into 3.19.5. After rebuild and reboot I can confirm this patch fixes my bug: irsend doesn't hang lircd. I'll report if anything else weird goes on, but this all seems pretty straightforward so I doubt I'll have any problems. > I'll send it out here shortly. If you could give it a spin at your > leisure, and if it works give me a Tested-by: tag I'd appreciate it! I'm not quite sure how to give a Tested-by, but from the minimal docs I found on the net, I am trying below (after your Signed-off-by tag). If I need to do something else, please point me in the general direction of instructions. > Great work again on chasing this down, and thanks for helping with > debugging and validating the fix! Thanks! I'm really glad this is getting worked out and am happy to help. It's a big step forward for me to move past simple bugzilla onto kernel bisects. I plan on giving a presentation about all this at my local Linux user group. It'll be nice to report the happy ending! You guys have been great. I guess kernel bugzilla isn't the place to get help: it's here on the LKML. P.S. Here's the kernel bz link, we/I can close it with the results once this is all done: https://bugzilla.kernel.org/show_bug.cgi?id=95431 > Cc: Nicolas Pitre > Cc: Thomas Gleixner > Cc: Josh Boyer > Cc: One Thousand Gnomes > Reported-by: Trevor Cordes > Signed-off-by: John Stultz Tested-by: Trevor Cordes (runtime test on i686) > --- > include/linux/ktime.h | 12 ++++++++++-- > kernel/time/hrtimer.c | 11 +++++++++-- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/include/linux/ktime.h b/include/linux/ktime.h > index 5fc3d10..d947263 100644 > --- a/include/linux/ktime.h > +++ b/include/linux/ktime.h > @@ -166,12 +166,20 @@ 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); > +extern s64 __ktime_divns(const ktime_t kt, s64 div); > static inline u64 ktime_divns(const ktime_t kt, s64 div) > { > if (__builtin_constant_p(div) && !(div >> 32)) { > - u64 ns = kt.tv64; > + s64 ns = kt.tv64; > + int neg = 0; > + > + if (ns < 0) { > + neg = 1; > + ns = -ns; > + } > do_div(ns, div); > + if (neg) > + ns = -ns; > return ns; > } else { > return __ktime_divns(kt, div); > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > index 76d4bd9..4c1b294 100644 > --- a/kernel/time/hrtimer.c > +++ b/kernel/time/hrtimer.c > @@ -266,12 +266,17 @@ 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; > + s64 dclc; > int sft = 0; > + int neg = 0; > > dclc = ktime_to_ns(kt); > + if (dclc < 0) { > + neg = 1; > + dclc = -dclc; > + } > /* Make sure the divisor is less than 2^32: */ > while (div >> 32) { > sft++; > @@ -279,6 +284,8 @@ u64 __ktime_divns(const ktime_t kt, s64 div) > } > dclc >>= sft; > do_div(dclc, (unsigned long) div); > + if (neg) > + dclc = -dclc; > > return dclc; > }