Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl")
This should have been marked extern inline in order to pick up the out of line definition in arch/x86/kernel/irqflags.S.
Cc: stable@vger.kernel.org # 4.18, 4.14, 4.9, 4.4 Reported-by: Ben Hutchings ben.hutchings@codethink.co.uk Signed-off-by: Nick Desaulniers ndesaulniers@google.com --- arch/x86/include/asm/irqflags.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c14f2a74b2be..15450a675031 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -33,7 +33,8 @@ extern inline unsigned long native_save_fl(void) return flags; }
-static inline void native_restore_fl(unsigned long flags) +extern inline void native_restore_fl(unsigned long flags); +extern inline void native_restore_fl(unsigned long flags) { asm volatile("push %0 ; popf" : /* no output */
On Mon, Aug 27, 2018 at 02:40:09PM -0700, Nick Desaulniers wrote:
Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl")
This should have been marked extern inline in order to pick up the out of line definition in arch/x86/kernel/irqflags.S.
Cc: stable@vger.kernel.org # 4.18, 4.14, 4.9, 4.4 Reported-by: Ben Hutchings ben.hutchings@codethink.co.uk Signed-off-by: Nick Desaulniers ndesaulniers@google.com
arch/x86/include/asm/irqflags.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c14f2a74b2be..15450a675031 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -33,7 +33,8 @@ extern inline unsigned long native_save_fl(void) return flags; } -static inline void native_restore_fl(unsigned long flags) +extern inline void native_restore_fl(unsigned long flags); +extern inline void native_restore_fl(unsigned long flags)
This looks odd to me, but my coffee hasn't kicked in yet this morning. Why do you need both lines here? Shouldn't the actual function be sufficient? If not, a comment explaining this would be nice.
thanks,
greg k-h
On 28/08/18 07:13, Greg Kroah-Hartman wrote:
On Mon, Aug 27, 2018 at 02:40:09PM -0700, Nick Desaulniers wrote:
Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl")
This should have been marked extern inline in order to pick up the out of line definition in arch/x86/kernel/irqflags.S.
Cc: stable@vger.kernel.org # 4.18, 4.14, 4.9, 4.4 Reported-by: Ben Hutchings ben.hutchings@codethink.co.uk Signed-off-by: Nick Desaulniers ndesaulniers@google.com
arch/x86/include/asm/irqflags.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c14f2a74b2be..15450a675031 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -33,7 +33,8 @@ extern inline unsigned long native_save_fl(void) return flags; } -static inline void native_restore_fl(unsigned long flags) +extern inline void native_restore_fl(unsigned long flags); +extern inline void native_restore_fl(unsigned long flags)
This looks odd to me, but my coffee hasn't kicked in yet this morning. Why do you need both lines here? Shouldn't the actual function be sufficient? If not, a comment explaining this would be nice.
Look just some lines further up:
/* 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)
Juergen
On Tue, Aug 28, 2018 at 08:02:37AM +0200, Juergen Gross wrote:
On 28/08/18 07:13, Greg Kroah-Hartman wrote:
On Mon, Aug 27, 2018 at 02:40:09PM -0700, Nick Desaulniers wrote:
Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl")
This should have been marked extern inline in order to pick up the out of line definition in arch/x86/kernel/irqflags.S.
Cc: stable@vger.kernel.org # 4.18, 4.14, 4.9, 4.4 Reported-by: Ben Hutchings ben.hutchings@codethink.co.uk Signed-off-by: Nick Desaulniers ndesaulniers@google.com
arch/x86/include/asm/irqflags.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c14f2a74b2be..15450a675031 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -33,7 +33,8 @@ extern inline unsigned long native_save_fl(void) return flags; } -static inline void native_restore_fl(unsigned long flags) +extern inline void native_restore_fl(unsigned long flags); +extern inline void native_restore_fl(unsigned long flags)
This looks odd to me, but my coffee hasn't kicked in yet this morning. Why do you need both lines here? Shouldn't the actual function be sufficient? If not, a comment explaining this would be nice.
Look just some lines further up:
/* 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)
Ugh, my fault, I'll go shut up now...
stupid gcc...
On Tue, Aug 28, 2018 at 5:43 AM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Tue, Aug 28, 2018 at 08:02:37AM +0200, Juergen Gross wrote:
On 28/08/18 07:13, Greg Kroah-Hartman wrote:
On Mon, Aug 27, 2018 at 02:40:09PM -0700, Nick Desaulniers wrote:
Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl")
This should have been marked extern inline in order to pick up the out of line definition in arch/x86/kernel/irqflags.S.
Cc: stable@vger.kernel.org # 4.18, 4.14, 4.9, 4.4 Reported-by: Ben Hutchings ben.hutchings@codethink.co.uk Signed-off-by: Nick Desaulniers ndesaulniers@google.com
arch/x86/include/asm/irqflags.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h index c14f2a74b2be..15450a675031 100644 --- a/arch/x86/include/asm/irqflags.h +++ b/arch/x86/include/asm/irqflags.h @@ -33,7 +33,8 @@ extern inline unsigned long native_save_fl(void) return flags; }
-static inline void native_restore_fl(unsigned long flags) +extern inline void native_restore_fl(unsigned long flags); +extern inline void native_restore_fl(unsigned long flags)
This looks odd to me, but my coffee hasn't kicked in yet this morning. Why do you need both lines here? Shouldn't the actual function be sufficient? If not, a comment explaining this would be nice.
That's a valid concern. I did hesitate while writing the commit message whether to describe that part or not. The reason to include the seemingly-additional-declaration was to prevent another case of commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl"). It doesn't hurt to provide more info in the commit message, so next time I'll prefer to explain more in my commit messages than less.
Thanks Juergen and Greg for reviewing the patch.
On 27/08/18 23:40, Nick Desaulniers wrote:
Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for native_save_fl")
This should have been marked extern inline in order to pick up the out of line definition in arch/x86/kernel/irqflags.S.
Cc: stable@vger.kernel.org # 4.18, 4.14, 4.9, 4.4 Reported-by: Ben Hutchings ben.hutchings@codethink.co.uk Signed-off-by: Nick Desaulniers ndesaulniers@google.com
Reviewed-by: Juergen Gross jgross@suse.com
Juergen
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Juergen Gross -
Nick Desaulniers