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.pi...@linaro.org>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Josh Boyer <jwbo...@redhat.com>
> Cc: One Thousand Gnomes <gno...@lxorguk.ukuu.org.uk>
> Reported-by: Trevor Cordes <tre...@tecnopolis.ca>
> Signed-off-by: John Stultz <john.stu...@linaro.org>
Tested-by: Trevor Cordes <tre...@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;
>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to