All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] perf/x86/intel: Make cpuc allocations consistent
@ 2019-03-14 21:10 Guenter Roeck
  2019-03-15  8:14 ` Peter Zijlstra
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2019-03-14 21:10 UTC (permalink / raw)
  To: Peter Zijlstra (Intel)
  Cc: Thomas Gleixner, stable, linux-kernel, Greg Kroah-Hartman

Hi,

On Tue, Mar 05, 2019 at 10:23:15PM +0100, Peter Zijlstra (Intel) wrote:
> The cpuc data structure allocation is different between fake and real
> cpuc's; use the same code to init/free both.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

With allmodconfig-CONFIG_CPU_SUP_INTEL, this patch results in:

In file included from arch/x86/events/amd/core.c:8:0:
arch/x86/events/amd/../perf_event.h:1036:45: warning: ‘struct cpu_hw_event’ declared inside parameter list will not be visible outside of this definition or declaration
 static inline int intel_cpuc_prepare(struct cpu_hw_event *cpuc, int cpu)
                                             ^~~~~~~~~~~~
arch/x86/events/amd/../perf_event.h:1041:45: warning: ‘struct cpu_hw_event’ declared inside parameter list will not be visible outside of this definition or declaration
 static inline void intel_cpuc_finish(struct cpu_hw_event *cpuc)
                                             ^~~~~~~~~~~~

With -Werror, this is fatal, and I think it may be buggy (should it be
cpu_hw_events) ?

The patch has been applied to stable releases. Any idea, anyone, why
this is the case ? It doesn't look like a bug fix to me, and reverting
it from v4.14.106 didn't seem to have a negative impact.

Guenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf/x86/intel: Make cpuc allocations consistent
  2019-03-14 21:10 [PATCH] perf/x86/intel: Make cpuc allocations consistent Guenter Roeck
@ 2019-03-15  8:14 ` Peter Zijlstra
  2019-03-15 12:16   ` [tip:perf/urgent] perf/x86: Fixup typo in stub functions tip-bot for Peter Zijlstra
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Zijlstra @ 2019-03-15  8:14 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Thomas Gleixner, stable, linux-kernel, Greg Kroah-Hartman, Ingo Molnar

On Thu, Mar 14, 2019 at 02:10:14PM -0700, Guenter Roeck wrote:
> On Tue, Mar 05, 2019 at 10:23:15PM +0100, Peter Zijlstra (Intel) wrote:
> > The cpuc data structure allocation is different between fake and real
> > cpuc's; use the same code to init/free both.
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> With allmodconfig-CONFIG_CPU_SUP_INTEL, this patch results in:
> 
> In file included from arch/x86/events/amd/core.c:8:0:
> arch/x86/events/amd/../perf_event.h:1036:45: warning: ‘struct cpu_hw_event’ declared inside parameter list will not be visible outside of this definition or declaration
>  static inline int intel_cpuc_prepare(struct cpu_hw_event *cpuc, int cpu)
>                                              ^~~~~~~~~~~~
> arch/x86/events/amd/../perf_event.h:1041:45: warning: ‘struct cpu_hw_event’ declared inside parameter list will not be visible outside of this definition or declaration
>  static inline void intel_cpuc_finish(struct cpu_hw_event *cpuc)
>                                              ^~~~~~~~~~~~
> 
> With -Werror, this is fatal, and I think it may be buggy (should it be
> cpu_hw_events) ?

Yep it should be. Won't result in a bug though, an unused pointer is an
unused poiner, no matter what known or unknown type it points to.

> The patch has been applied to stable releases. Any idea, anyone, why
> this is the case ? It doesn't look like a bug fix to me, and reverting
> it from v4.14.106 didn't seem to have a negative impact.

Reverting just this one patch will make your kernel go boom when you run
on a skylake or more recent Intel cpu, have updated your microcode and
do:

  echo 0 > /sys/bus/event_source/devices/cpu/allow_tsx_force_abort

And then try and create perf events. See also commit:

  400816f60c54 ("perf/x86/intel: Implement support for TSX Force Abort")

---
Subject: perf/x86: Fixup typo in stub functions

Guenter reported a build warning for CONFIG_CPU_SUP_INTEL=n. While
harmless (an unsed pointer is an unused pointer, no matter the type) it
needs fixing.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Fixes: d01b1f96a82e ("perf/x86/intel: Make cpuc allocations consistent")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/events/perf_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index b04ae6c8775e..a75955741c50 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1033,12 +1033,12 @@ static inline int intel_pmu_init(void)
 	return 0;
 }
 
-static inline int intel_cpuc_prepare(struct cpu_hw_event *cpuc, int cpu)
+static inline int intel_cpuc_prepare(struct cpu_hw_events *cpuc, int cpu)
 {
 	return 0;
 }
 
-static inline void intel_cpuc_finish(struct cpu_hw_event *cpuc)
+static inline void intel_cpuc_finish(struct cpu_hw_events *cpuc)
 {
 }
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tip:perf/urgent] perf/x86: Fixup typo in stub functions
  2019-03-15  8:14 ` Peter Zijlstra
@ 2019-03-15 12:16   ` tip-bot for Peter Zijlstra
  0 siblings, 0 replies; 3+ messages in thread
From: tip-bot for Peter Zijlstra @ 2019-03-15 12:16 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, peterz, linux-kernel, linux, gregkh, hpa, mingo, tglx

Commit-ID:  f764c58b7faa26f5714e6907f892abc2bc0de4f8
Gitweb:     https://git.kernel.org/tip/f764c58b7faa26f5714e6907f892abc2bc0de4f8
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Fri, 15 Mar 2019 09:14:10 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 15 Mar 2019 13:12:42 +0100

perf/x86: Fixup typo in stub functions

Guenter reported a build warning for CONFIG_CPU_SUP_INTEL=n:

  > With allmodconfig-CONFIG_CPU_SUP_INTEL, this patch results in:
  >
  > In file included from arch/x86/events/amd/core.c:8:0:
  > arch/x86/events/amd/../perf_event.h:1036:45: warning: ‘struct cpu_hw_event’ declared inside parameter list will not be visible outside of this definition or declaration
  >  static inline int intel_cpuc_prepare(struct cpu_hw_event *cpuc, int cpu)

While harmless (an unsed pointer is an unused pointer, no matter the type)
it needs fixing.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Fixes: d01b1f96a82e ("perf/x86/intel: Make cpuc allocations consistent")
Link: http://lkml.kernel.org/r/20190315081410.GR5996@hirez.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/perf_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index b04ae6c8775e..a75955741c50 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1033,12 +1033,12 @@ static inline int intel_pmu_init(void)
 	return 0;
 }
 
-static inline int intel_cpuc_prepare(struct cpu_hw_event *cpuc, int cpu)
+static inline int intel_cpuc_prepare(struct cpu_hw_events *cpuc, int cpu)
 {
 	return 0;
 }
 
-static inline void intel_cpuc_finish(struct cpu_hw_event *cpuc)
+static inline void intel_cpuc_finish(struct cpu_hw_events *cpuc)
 {
 }
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-03-15 12:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-14 21:10 [PATCH] perf/x86/intel: Make cpuc allocations consistent Guenter Roeck
2019-03-15  8:14 ` Peter Zijlstra
2019-03-15 12:16   ` [tip:perf/urgent] perf/x86: Fixup typo in stub functions tip-bot for Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.