Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1587959

Re: gcc7 log2 compile issues in kernel/time/timekeeping.c

Show all headers | View raw

On 24 February 2017 at 21:25, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Feb 23, 2017 at 10:43 AM, Laura Abbott <labbott@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 :       \

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: gcc7 log2 compile issues in kernel/time/timekeeping.c John Stultz <john.stultz@linaro.org> - 2017-02-24 22:30 +0100
  Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-24 22:50 +0100
    Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Laura Abbott <labbott@redhat.com> - 2017-02-25 00:40 +0100
      Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Markus Trippelsdorf <markus@trippelsdorf.de> - 2017-02-25 09:20 +0100
        Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-25 10:20 +0100
          Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Markus Trippelsdorf <markus@trippelsdorf.de> - 2017-02-25 12:20 +0100
            Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-25 12:40 +0100
              Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-25 13:00 +0100
                Re: gcc7 log2 compile issues in kernel/time/timekeeping.c Laura Abbott <labbott@redhat.com> - 2017-03-01 02:20 +0100

csiph-web