From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751403AbdBXXdm (ORCPT ); Fri, 24 Feb 2017 18:33:42 -0500 Received: from mail-qt0-f175.google.com ([209.85.216.175]:32999 "EHLO mail-qt0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750831AbdBXXdl (ORCPT ); Fri, 24 Feb 2017 18:33:41 -0500 Subject: Re: gcc7 log2 compile issues in kernel/time/timekeeping.c To: Ard Biesheuvel , John Stultz References: <9c74d635-d0d1-0893-8093-ce20b0933fc7@redhat.com> Cc: Thomas Gleixner , Linux Kernel Mailing List From: Laura Abbott Message-ID: <51e768e7-91af-86c5-3732-2e529364d376@redhat.com> Date: Fri, 24 Feb 2017 15:33:37 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/24/2017 01:45 PM, Ard Biesheuvel wrote: > On 24 February 2017 at 21:25, John Stultz wrote: >> On Thu, Feb 23, 2017 at 10:43 AM, Laura Abbott 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