On 02/24/2017 01:45 PM, Ard Biesheuvel wrote: > On 24 February 2017 at 21:25, John Stultz <john.stu...@linaro.org> wrote: >> On Thu, Feb 23, 2017 at 10:43 AM, Laura Abbott <labb...@redhat.com> wrote: >>> Hi, >>> >>> Fedora was previously carrying a workaround for a gcc7 issue reported >>> on arm64 >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461597.html. >>> The workaround got rid of __ilog2_NaN. I dropped the patch this morning >>> because a proper fix (29905b52fad0 ("log2: make order_base_2() behave >>> correctly on const input value zero")) was merged. This fixed the arm64 >>> problem linked in the thread but there seems to be another issue in >>> timekeeping.c: >>> >>> /kernel/time/timekeeping.c:2051: undefined reference to `____ilog2_NaN' >>> >>> Fedora enables CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE so I think the >>> compiler is calculating a possible constant of 0 once again. >>> >>> Any ideas about a proper fix? >> >> Huh. So if I understand this, its because we don't explicit checks for >> offsec or cycle_interval being zero in: >> >> shift = ilog2(offset) - ilog2(tk->cycle_interval); >> >> Right? >> >> Clearly that case isn't expected to happen, but if it did we'd want >> the result of ilog2 to return zero. So I'm not sure if that >> order_base_2() function is maybe the right function to use as it has >> an explict zero check? >> > > The problem is really that GCC splits off a constant folded clone > where one of these variables is a constant 0. In the order_base_2() > case, we could sidestep it by fixing an existing issue with the > function itself, but in this case, it is ilog2() itself that is > affected. > > Laura, does the below make any difference at all? > > diff --git a/include/linux/log2.h b/include/linux/log2.h > index fd7ff3d91e6a..cf4e5bb662bd 100644 > --- a/include/linux/log2.h > +++ b/include/linux/log2.h > @@ -85,7 +85,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n) > #define ilog2(n) \ > ( \ > __builtin_constant_p(n) ? ( \ > - (n) < 1 ? ____ilog2_NaN() : \ > + __builtin_expect((n) < 1, 0) ? \ > + ____ilog2_NaN() : \ > (n) & (1ULL << 63) ? 63 : \ > (n) & (1ULL << 62) ? 62 : \ > (n) & (1ULL << 61) ? 61 : \ >
No, still see the same issue. Thanks, Laura
