* native_save_fl() causes a warning @ 2018-08-03 13:10 Jean Delvare 2018-08-03 16:38 ` Nick Desaulniers 0 siblings, 1 reply; 11+ messages in thread From: Jean Delvare @ 2018-08-03 13:10 UTC (permalink / raw) To: Nick Desaulniers Cc: LKML, Alistair Strachan, Matthias Kaehlcke, Arnd Bergmann, H. Peter Anvin, Tom Stellar, Sedat Dilek, Juergen Gross, Ingo Molnar 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=1): ./arch/x86/include/asm/irqflags.h:16:29: warning: no previous prototype for ‘native_save_fl’ [-Wmissing-prototypes] extern inline unsigned long native_save_fl(void) ^ Please fix it. 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? Thanks, -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: native_save_fl() causes a warning 2018-08-03 13:10 native_save_fl() causes a warning Jean Delvare @ 2018-08-03 16:38 ` Nick Desaulniers 2018-08-03 17:05 ` [PATCH] x86/irqflags: provide a declaration for native_save_fl Nick Desaulniers 2018-08-03 17:49 ` native_save_fl() causes a warning Alistair Strachan 0 siblings, 2 replies; 11+ messages in thread From: Nick Desaulniers @ 2018-08-03 16:38 UTC (permalink / raw) To: jdelvare Cc: LKML, Alistair Strachan, Matthias Kaehlcke, Arnd Bergmann, hpa, tstellar, sedat.dilek, jgross, Ingo Molnar, David.Laight 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=1): > > ./arch/x86/include/asm/irqflags.h:16:29: warning: no previous prototype for ‘native_save_fl’ [-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. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] x86/irqflags: provide a declaration for native_save_fl 2018-08-03 16:38 ` Nick Desaulniers @ 2018-08-03 17:05 ` Nick Desaulniers 2018-08-05 20:33 ` [tip:x86/urgent] x86/irqflags: Provide " tip-bot for Nick Desaulniers 2018-08-06 10:25 ` [PATCH] x86/irqflags: provide " David Laight 2018-08-03 17:49 ` native_save_fl() causes a warning Alistair Strachan 1 sibling, 2 replies; 11+ messages in thread From: Nick Desaulniers @ 2018-08-03 17:05 UTC (permalink / raw) To: tglx, mingo, hpa Cc: x86, jgross, kstewart, gregkh, boris.ostrovsky, linux-kernel, jdelvare, astrachan, mka, arnd, tstellar, sedat.dilek, David.Laight, Nick Desaulniers, stable Fixes commit d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern inline"). It was reported that the above commit was causing users of gcc < 4.9 to observe -Werror=missing-prototypes errors. Indeed, it seems that: extern inline unsigned long native_save_fl(void) { return 0; } compiled with -Werror=missing-prototypes produces this warning in gcc < 4.9, but not gcc >= 4.9. Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Reported-by: David Laight <david.laight@aculab.com> Reported-by: Jean Delvare <jdelvare@suse.de> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- arch/x86/include/asm/irqflags.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c4fc17220df9..c14f2a74b2be 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -13,6 +13,8 @@ * Interrupt control: */ +/* Declaration required for gcc < 4.9 to prevent -Werror=missing-prototypes */ +extern inline unsigned long native_save_fl(void); extern inline unsigned long native_save_fl(void) { unsigned long flags; -- 2.18.0.597.ga71716f1ad-goog ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:x86/urgent] x86/irqflags: Provide a declaration for native_save_fl 2018-08-03 17:05 ` [PATCH] x86/irqflags: provide a declaration for native_save_fl Nick Desaulniers @ 2018-08-05 20:33 ` tip-bot for Nick Desaulniers 2018-08-06 21:33 ` Nick Desaulniers 2018-08-06 10:25 ` [PATCH] x86/irqflags: provide " David Laight 1 sibling, 1 reply; 11+ messages in thread From: tip-bot for Nick Desaulniers @ 2018-08-05 20:33 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, jdelvare, ndesaulniers, hpa, tglx, david.laight, linux-kernel Commit-ID: 208cbb32558907f68b3b2a081ca2337ac3744794 Gitweb: https://git.kernel.org/tip/208cbb32558907f68b3b2a081ca2337ac3744794 Author: Nick Desaulniers <ndesaulniers@google.com> AuthorDate: Fri, 3 Aug 2018 10:05:50 -0700 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Sun, 5 Aug 2018 22:30:37 +0200 x86/irqflags: Provide a declaration for native_save_fl It was reported that the commit d0a8d9378d16 is causing users of gcc < 4.9 to observe -Werror=missing-prototypes errors. Indeed, it seems that: extern inline unsigned long native_save_fl(void) { return 0; } compiled with -Werror=missing-prototypes produces this warning in gcc < 4.9, but not gcc >= 4.9. Fixes: d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern inline"). Reported-by: David Laight <david.laight@aculab.com> Reported-by: Jean Delvare <jdelvare@suse.de> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: hpa@zytor.com Cc: jgross@suse.com Cc: kstewart@linuxfoundation.org Cc: gregkh@linuxfoundation.org Cc: boris.ostrovsky@oracle.com Cc: astrachan@google.com Cc: mka@chromium.org Cc: arnd@arndb.de Cc: tstellar@redhat.com Cc: sedat.dilek@gmail.com Cc: David.Laight@aculab.com Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/20180803170550.164688-1-ndesaulniers@google.com --- arch/x86/include/asm/irqflags.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c4fc17220df9..c14f2a74b2be 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -13,6 +13,8 @@ * Interrupt control: */ +/* Declaration required for gcc < 4.9 to prevent -Werror=missing-prototypes */ +extern inline unsigned long native_save_fl(void); extern inline unsigned long native_save_fl(void) { unsigned long flags; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [tip:x86/urgent] x86/irqflags: Provide a declaration for native_save_fl 2018-08-05 20:33 ` [tip:x86/urgent] x86/irqflags: Provide " tip-bot for Nick Desaulniers @ 2018-08-06 21:33 ` Nick Desaulniers 2018-08-07 7:29 ` Thomas Gleixner 0 siblings, 1 reply; 11+ messages in thread From: Nick Desaulniers @ 2018-08-06 21:33 UTC (permalink / raw) To: hpa, Thomas Gleixner, david.laight, LKML, jdelvare, Ingo Molnar Cc: linux-tip-commits, sedat.dilek On Sun, Aug 5, 2018 at 1:33 PM tip-bot for Nick Desaulniers <tipbot@zytor.com> wrote: > > Commit-ID: 208cbb32558907f68b3b2a081ca2337ac3744794 > Gitweb: https://git.kernel.org/tip/208cbb32558907f68b3b2a081ca2337ac3744794 > Author: Nick Desaulniers <ndesaulniers@google.com> > AuthorDate: Fri, 3 Aug 2018 10:05:50 -0700 > Committer: Thomas Gleixner <tglx@linutronix.de> > CommitDate: Sun, 5 Aug 2018 22:30:37 +0200 > > x86/irqflags: Provide a declaration for native_save_fl > > It was reported that the commit d0a8d9378d16 is causing users of gcc < 4.9 > to observe -Werror=missing-prototypes errors. > > Indeed, it seems that: > extern inline unsigned long native_save_fl(void) { return 0; } > > compiled with -Werror=missing-prototypes produces this warning in gcc < > 4.9, but not gcc >= 4.9. > > Fixes: d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern inline"). > Reported-by: David Laight <david.laight@aculab.com> > Reported-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: hpa@zytor.com > Cc: jgross@suse.com > Cc: kstewart@linuxfoundation.org > Cc: gregkh@linuxfoundation.org > Cc: boris.ostrovsky@oracle.com > Cc: astrachan@google.com > Cc: mka@chromium.org > Cc: arnd@arndb.de > Cc: tstellar@redhat.com > Cc: sedat.dilek@gmail.com > Cc: David.Laight@aculab.com > Cc: stable@vger.kernel.org Not sure if this was going to be cleaned up in an automated way, but looks like this commit message drops the comment to stable as to how far back it should go: # 4.17, 4.14, 4.9, 4.4 also, there were tested by's reported: Tested-by: David Laight <david.laight@acuab.com> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [tip:x86/urgent] x86/irqflags: Provide a declaration for native_save_fl 2018-08-06 21:33 ` Nick Desaulniers @ 2018-08-07 7:29 ` Thomas Gleixner 2018-08-07 16:46 ` Nick Desaulniers 0 siblings, 1 reply; 11+ messages in thread From: Thomas Gleixner @ 2018-08-07 7:29 UTC (permalink / raw) To: Nick Desaulniers Cc: hpa, david.laight, LKML, jdelvare, Ingo Molnar, linux-tip-commits, sedat.dilek On Mon, 6 Aug 2018, Nick Desaulniers wrote: > On Sun, Aug 5, 2018 at 1:33 PM tip-bot for Nick Desaulniers > <tipbot@zytor.com> wrote: > > Cc: stable@vger.kernel.org > > Not sure if this was going to be cleaned up in an automated way, but > looks like this commit message drops the comment to stable as to how > far back it should go: > > # 4.17, 4.14, 4.9, 4.4 The Fixes tag is enough. If the upstream commit was backported, then the tools of the stable folks will find it and know exactly how far it needs to go back. > also, there were tested by's reported: > > Tested-by: David Laight <david.laight@acuab.com> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> Which came in after I applied it.... Thanks, tglx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [tip:x86/urgent] x86/irqflags: Provide a declaration for native_save_fl 2018-08-07 7:29 ` Thomas Gleixner @ 2018-08-07 16:46 ` Nick Desaulniers 2018-08-07 16:57 ` Thomas Gleixner 0 siblings, 1 reply; 11+ messages in thread From: Nick Desaulniers @ 2018-08-07 16:46 UTC (permalink / raw) To: Thomas Gleixner Cc: hpa, david.laight, LKML, jdelvare, Ingo Molnar, linux-tip-commits, sedat.dilek On Tue, Aug 7, 2018 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, 6 Aug 2018, Nick Desaulniers wrote: > > On Sun, Aug 5, 2018 at 1:33 PM tip-bot for Nick Desaulniers > > <tipbot@zytor.com> wrote: > > > Cc: stable@vger.kernel.org > > > > Not sure if this was going to be cleaned up in an automated way, but > > looks like this commit message drops the comment to stable as to how > > far back it should go: > > > > # 4.17, 4.14, 4.9, 4.4 > > The Fixes tag is enough. If the upstream commit was backported, then the > tools of the stable folks will find it and know exactly how far it needs to > go back. Oh, cool. > > also, there were tested by's reported: > > > > Tested-by: David Laight <david.laight@acuab.com> > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > Which came in after I applied it.... I've seen other maintainers revise commit messages before sending pull requests along, but I guess that's problematic as anyone else who has pulled before the revision would then have a merge conflict. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [tip:x86/urgent] x86/irqflags: Provide a declaration for native_save_fl 2018-08-07 16:46 ` Nick Desaulniers @ 2018-08-07 16:57 ` Thomas Gleixner 0 siblings, 0 replies; 11+ messages in thread From: Thomas Gleixner @ 2018-08-07 16:57 UTC (permalink / raw) To: Nick Desaulniers Cc: hpa, david.laight, LKML, jdelvare, Ingo Molnar, linux-tip-commits, sedat.dilek On Tue, 7 Aug 2018, Nick Desaulniers wrote: > On Tue, Aug 7, 2018 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, 6 Aug 2018, Nick Desaulniers wrote: > > > also, there were tested by's reported: > > > > > > Tested-by: David Laight <david.laight@acuab.com> > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > Which came in after I applied it.... > > I've seen other maintainers revise commit messages before sending pull > requests along, but I guess that's problematic as anyone else who has > pulled before the revision would then have a merge conflict. In general I avoid redoing commits for that reason and others and one of the reasons why we have the Link: tag in the commit message is to be able to look up such additional information easily - assumed that the archives have not vanished from the planet. Thanks, tglx ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] x86/irqflags: provide a declaration for native_save_fl 2018-08-03 17:05 ` [PATCH] x86/irqflags: provide a declaration for native_save_fl Nick Desaulniers 2018-08-05 20:33 ` [tip:x86/urgent] x86/irqflags: Provide " tip-bot for Nick Desaulniers @ 2018-08-06 10:25 ` David Laight 2018-08-06 10:38 ` Sedat Dilek 1 sibling, 1 reply; 11+ messages in thread From: David Laight @ 2018-08-06 10:25 UTC (permalink / raw) To: 'Nick Desaulniers', tglx, mingo, hpa Cc: x86, jgross, kstewart, gregkh, boris.ostrovsky, linux-kernel, jdelvare, astrachan, mka, arnd, tstellar, sedat.dilek, stable From: Nick Desaulniers > Sent: 03 August 2018 18:06 > Fixes commit d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern > inline"). > > It was reported that the above commit was causing users of gcc < 4.9 to > observe -Werror=missing-prototypes errors. > > Indeed, it seems that: > extern inline unsigned long native_save_fl(void) { return 0; } > > compiled with -Werror=missing-prototypes produces this warning in gcc < > 4.9, but not gcc >= 4.9. > > Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 > Reported-by: David Laight <david.laight@aculab.com> > Reported-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> You can add my: Tested-by: David Laight <david.laight@acuab.com> David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86/irqflags: provide a declaration for native_save_fl 2018-08-06 10:25 ` [PATCH] x86/irqflags: provide " David Laight @ 2018-08-06 10:38 ` Sedat Dilek 0 siblings, 0 replies; 11+ messages in thread From: Sedat Dilek @ 2018-08-06 10:38 UTC (permalink / raw) To: David Laight Cc: Nick Desaulniers, tglx, mingo, hpa, x86, jgross, kstewart, gregkh, boris.ostrovsky, linux-kernel, jdelvare, astrachan, mka, arnd, tstellar, stable On Mon, Aug 6, 2018 at 12:25 PM, David Laight <David.Laight@aculab.com> wrote: > From: Nick Desaulniers >> Sent: 03 August 2018 18:06 >> Fixes commit d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern >> inline"). >> >> It was reported that the above commit was causing users of gcc < 4.9 to >> observe -Werror=missing-prototypes errors. >> >> Indeed, it seems that: >> extern inline unsigned long native_save_fl(void) { return 0; } >> >> compiled with -Werror=missing-prototypes produces this warning in gcc < >> 4.9, but not gcc >= 4.9. >> >> Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 >> Reported-by: David Laight <david.laight@aculab.com> >> Reported-by: Jean Delvare <jdelvare@suse.de> >> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > You can add my: > > Tested-by: David Laight <david.laight@acuab.com> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> On top of Linux v4.18-rc8 with clang version 7.0.0-svn338205. - sed@ - ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: native_save_fl() causes a warning 2018-08-03 16:38 ` Nick Desaulniers 2018-08-03 17:05 ` [PATCH] x86/irqflags: provide a declaration for native_save_fl Nick Desaulniers @ 2018-08-03 17:49 ` Alistair Strachan 1 sibling, 0 replies; 11+ messages in thread From: Alistair Strachan @ 2018-08-03 17:49 UTC (permalink / raw) To: Nick Desaulniers Cc: jdelvare, linux-kernel, mka, Arnd Bergmann, hpa, tstellar, sedat.dilek, jgross, mingo, David.Laight On Fri, Aug 3, 2018 at 9:38 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > 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=1): > > > > ./arch/x86/include/asm/irqflags.h:16:29: warning: no previous prototype for ‘native_save_fl’ [-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? The core issue these patches worked around was the automatic/heuristic generation of stack guard code by clang, which ended up clobbering %ecx/%rcx in a way not expected by the contract of the paravirt code. The only function affected by this problem was native_save_fl(), because only it has a C stack (and thus, has a stack that requires guarding). The other functions could have been converted at the same time, and they will have to be converted if (down the line) somebody adds C stack variables to them. But, for now, the patch series seems to correctly work around this issue. > 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. This bug was so obvious you just enabled CONFIG_PARAVIRT, built with CC=clang, and booted the x86_64 bzImage on any qemu. The minute the paravirt alternatives were patched in, it exploded. > -- > Thanks, > ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-08-07 16:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-03 13:10 native_save_fl() causes a warning Jean Delvare 2018-08-03 16:38 ` Nick Desaulniers 2018-08-03 17:05 ` [PATCH] x86/irqflags: provide a declaration for native_save_fl Nick Desaulniers 2018-08-05 20:33 ` [tip:x86/urgent] x86/irqflags: Provide " tip-bot for Nick Desaulniers 2018-08-06 21:33 ` Nick Desaulniers 2018-08-07 7:29 ` Thomas Gleixner 2018-08-07 16:46 ` Nick Desaulniers 2018-08-07 16:57 ` Thomas Gleixner 2018-08-06 10:25 ` [PATCH] x86/irqflags: provide " David Laight 2018-08-06 10:38 ` Sedat Dilek 2018-08-03 17:49 ` native_save_fl() causes a warning Alistair Strachan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).