Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180803151035.238a19d2@endymion>
In-Reply-To: <20180803151035.238a19d2@endymion>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Fri, 3 Aug 2018 09:38:03 -0700
Message-ID: <CAKwvOdmBtzkca-C1YpB_OYVyqFFi3P_Q8DJPaKr96DFZF0e6Hg@mail.gmail.com>
Subject: Re: native_save_fl() causes a warning
To:     jdelvare@suse.de
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Alistair Strachan <astrachan@google.com>,
        Matthias Kaehlcke <mka@chromium.org>,
        Arnd Bergmann <arnd@arndb.de>, hpa@zytor.com,
        tstellar@redhat.com, sedat.dilek@gmail.com, jgross@suse.com,
        Ingo Molnar <mingo@kernel.org>, David.Laight@aculab.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 3, 2018 at 6:10 AM Jean Delvare <jdelvare@suse.de> wrote:
>
> Hi Nick,
>
> It seems that this linux kernel commit of yours:
>
> commit d0a8d9378d16eb3c69bd8e6d23779fbdbee3a8c7
> Author: Nick Desaulniers
> Date:   Thu Jun 21 09:23:24 2018 -0700
>
>     x86/paravirt: Make native_save_fl() extern inline
>
> introduced a new warning (with W=3D1):
>
> ./arch/x86/include/asm/irqflags.h:16:29: warning: no previous prototype f=
or =E2=80=98native_save_fl=E2=80=99 [-Wmissing-prototypes]
>  extern inline unsigned long native_save_fl(void)
>                              ^
>
> Please fix it.

Hi Jean, thanks for the report.  David Laight also reported this
warning; he tested a patch I sent him overnight.

Let me guess, you're using a version of GCC < 4.9?  It seems that GCC
< 4.9 will produce that warning for extern inline functions without
previous declarations.

I'll add your Reported-By tag to the patch that I will send out in a
few minutes.

> Secondly, I am quite curious why you changed only native_save_fl() from
> static inline to extern inline, when native_restore_fl(),
> native_irq_disable() and native_irq_enable() are equally referenced by
> address in arch/x86/kernel/paravirt.c and thus should suffer from the
> same problem. Can you explain?

This is a good point.  With native_save_fl, we were not able to boot
the kernel at all.  Maybe this was called from the boot sequence
(maybe Juergen knows more)?  It seems that the other functions aren't
preventing us from booting, but maybe exercising other paths in
paravirt would expose such an issue?  Or maybe paravirt doesn't have
the same calling convention requirements for those functions?

Is there a standard testing procedure for paravirt? I'd be happy to
try it to see if we can expose more things that should have the same
cleanup.
--=20
Thanks,
~Nick Desaulniers