Messages in this thread | | | Date | Sat, 2 May 2015 03:31:56 -0500 | From | Trevor Cordes <> | Subject | Re: [PATCH] ktime: Fix ktime_divns to do signed division |
| |
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 <nicolas.pitre@linaro.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Josh Boyer <jwboyer@redhat.com> > Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> > Reported-by: Trevor Cordes <trevor@tecnopolis.ca> > Signed-off-by: John Stultz <john.stultz@linaro.org> Tested-by: Trevor Cordes <trevor@tecnopolis.ca> (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; > }
| |