Patchwork x86/mce: Streamline MCE subsystem's naming

login
register
mail settings
Submitter Borislav Petkov
Date Dec. 5, 2018, 2:13 p.m.
Message ID <20181205141323.14995-1-bp@alien8.de>
Download mbox | patch
Permalink /patch/673153/
State New
Headers show

Comments

Borislav Petkov - Dec. 5, 2018, 2:13 p.m.
From: Borislav Petkov <bp@suse.de>

Rename the containing folder to "mce" which is the most widespread name.
Drop the "mce[-_]" filename prefix of some compilation units (while
others don't have it).

This unifies the file naming in the MCE subsystem:

mce/
|-- amd.c
|-- apei.c
|-- core.c
|-- dev-mcelog.c
|-- genpool.c
|-- inject.c
|-- intel.c
|-- internal.h
|-- Makefile
|-- p5.c
|-- severity.c
|-- therm_throt.c
|-- threshold.c
`-- winchip.c

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/Makefile                           |  2 +-
 arch/x86/kernel/cpu/{mcheck => mce}/Makefile           | 10 +++++-----
 arch/x86/kernel/cpu/{mcheck/mce_amd.c => mce/amd.c}    |  2 +-
 arch/x86/kernel/cpu/{mcheck/mce-apei.c => mce/apei.c}  |  2 +-
 arch/x86/kernel/cpu/{mcheck/mce.c => mce/core.c}       |  2 +-
 arch/x86/kernel/cpu/{mcheck => mce}/dev-mcelog.c       |  2 +-
 .../kernel/cpu/{mcheck/mce-genpool.c => mce/genpool.c} |  2 +-
 .../kernel/cpu/{mcheck/mce-inject.c => mce/inject.c}   |  2 +-
 .../x86/kernel/cpu/{mcheck/mce_intel.c => mce/intel.c} |  2 +-
 .../cpu/{mcheck/mce-internal.h => mce/internal.h}      |  0
 arch/x86/kernel/cpu/{mcheck => mce}/p5.c               |  0
 .../cpu/{mcheck/mce-severity.c => mce/severity.c}      |  2 +-
 arch/x86/kernel/cpu/{mcheck => mce}/therm_throt.c      |  0
 arch/x86/kernel/cpu/{mcheck => mce}/threshold.c        |  0
 arch/x86/kernel/cpu/{mcheck => mce}/winchip.c          |  0
 15 files changed, 14 insertions(+), 14 deletions(-)
 rename arch/x86/kernel/cpu/{mcheck => mce}/Makefile (52%)
 rename arch/x86/kernel/cpu/{mcheck/mce_amd.c => mce/amd.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck/mce-apei.c => mce/apei.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck/mce.c => mce/core.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck => mce}/dev-mcelog.c (99%)
 rename arch/x86/kernel/cpu/{mcheck/mce-genpool.c => mce/genpool.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck/mce-inject.c => mce/inject.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck/mce_intel.c => mce/intel.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck/mce-internal.h => mce/internal.h} (100%)
 rename arch/x86/kernel/cpu/{mcheck => mce}/p5.c (100%)
 rename arch/x86/kernel/cpu/{mcheck/mce-severity.c => mce/severity.c} (99%)
 rename arch/x86/kernel/cpu/{mcheck => mce}/therm_throt.c (100%)
 rename arch/x86/kernel/cpu/{mcheck => mce}/threshold.c (100%)
 rename arch/x86/kernel/cpu/{mcheck => mce}/winchip.c (100%)
Ingo Molnar - Dec. 5, 2018, 4:30 p.m.
* Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> Rename the containing folder to "mce" which is the most widespread name.
> Drop the "mce[-_]" filename prefix of some compilation units (while
> others don't have it).
> 
> This unifies the file naming in the MCE subsystem:
> 
> mce/
> |-- amd.c
> |-- apei.c
> |-- core.c
> |-- dev-mcelog.c
> |-- genpool.c
> |-- inject.c
> |-- intel.c
> |-- internal.h
> |-- Makefile
> |-- p5.c
> |-- severity.c
> |-- therm_throt.c
> |-- threshold.c
> `-- winchip.c
> 
> No functional changes.

Cool!

Would it make sense to organize it a bit more and separate out vendor 
specific functionality:

  mce/cpu/intel.c
  mce/cpu/intel-p5.c
  mce/cpu/amd.c
  mce/cpu/winchip.c

  mce/internal.h
  mce/core.c

  mce/genpool.c
  mce/threshold.c
  mce/severity.c
  mce/inject.c
  mce/therm_throt.c
  mce/dev-mcelog.c
  mce/apei.c

?

This way there's a clear separation between low level, vendor specific 
MCE logic and higher level MCE logic.

mce/apei.c, if this is an Intel-only feature, could perhaps become 
mce/cpu/intel-apei.c?

Anyway, your patch is fine too, so whichever subset you decide to use:

  Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo
Luck, Tony - Dec. 5, 2018, 4:57 p.m.
On Wed, Dec 05, 2018 at 03:13:23PM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Rename the containing folder to "mce" which is the most widespread name.
> Drop the "mce[-_]" filename prefix of some compilation units (while
> others don't have it).
> 
> This unifies the file naming in the MCE subsystem:
> 
> mce/
> |-- amd.c
> |-- apei.c
> |-- core.c
> |-- dev-mcelog.c
> |-- genpool.c
> |-- inject.c
> |-- intel.c
> |-- internal.h
> |-- Makefile
> |-- p5.c
> |-- severity.c
> |-- therm_throt.c
> |-- threshold.c
> `-- winchip.c
> 
> No functional changes.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>

Acked-by: Tony Luck <tony.luck@intel.com>
Borislav Petkov - Dec. 5, 2018, 5 p.m.
On Wed, Dec 05, 2018 at 05:30:37PM +0100, Ingo Molnar wrote:
> Would it make sense to organize it a bit more and separate out vendor 
> specific functionality:
> 
>   mce/cpu/intel.c
>   mce/cpu/intel-p5.c
>   mce/cpu/amd.c
>   mce/cpu/winchip.c

That's too fine-grained IMO and look at the path we'd get then:

arch/x86/kernel/cpu/mce/cpu/intel.c
		^^^     ^^^

which brings me to something we already talked about: the "kernel" part
of the arch/x86/ paths. See this thread:

https://lkml.kernel.org/r/20140114185802.GB29871@pd.tnic

from 2014. We practically agreed there that "kernel/" is redundant as it
all is kernel. So maybe we should start moving stuff up into arch/x86/
and then kill kernel/ eventually.

> This way there's a clear separation between low level, vendor specific 
> MCE logic and higher level MCE logic.
> 
> mce/apei.c, if this is an Intel-only feature, could perhaps become 
> mce/cpu/intel-apei.c?

Yeah, I think the pile in mce/ is pretty succinct now. We can always
separate it more later, if it starts to hurt but right now it is ok,
IMO.

> Anyway, your patch is fine too, so whichever subset you decide to use:
> 
>   Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thx.
Ingo Molnar - Dec. 5, 2018, 6:01 p.m.
* Borislav Petkov <bp@alien8.de> wrote:

> On Wed, Dec 05, 2018 at 05:30:37PM +0100, Ingo Molnar wrote:
> > Would it make sense to organize it a bit more and separate out vendor 
> > specific functionality:
> > 
> >   mce/cpu/intel.c
> >   mce/cpu/intel-p5.c
> >   mce/cpu/amd.c
> >   mce/cpu/winchip.c
> 
> That's too fine-grained IMO and look at the path we'd get then:
> 
> arch/x86/kernel/cpu/mce/cpu/intel.c
> 		^^^     ^^^

Oh - I thought we'd have arch/x86/mce/ or so?

There's machine check events that are not tied to any particular CPU, 
correct? So this would be the right conceptual level - and it would also 
remove the somewhat redundant 'kernel' part.

Thanks,

	Ingo
Borislav Petkov - Dec. 5, 2018, 7:03 p.m.
On Wed, Dec 05, 2018 at 07:01:58PM +0100, Ingo Molnar wrote:
> Oh - I thought we'd have arch/x86/mce/ or so?
> 
> There's machine check events that are not tied to any particular CPU, 
> correct? So this would be the right conceptual level - and it would also 
> remove the somewhat redundant 'kernel' part.

Well, all the MCE events reported are some way or the other tied to the
CPU and they're reported in the CPU's MCA banks so I think we want

	arch/x86/cpu/mce/
Ingo Molnar - Dec. 6, 2018, 4:27 p.m.
* Borislav Petkov <bp@alien8.de> wrote:

> On Wed, Dec 05, 2018 at 07:01:58PM +0100, Ingo Molnar wrote:
> > Oh - I thought we'd have arch/x86/mce/ or so?
> > 
> > There's machine check events that are not tied to any particular CPU, 
> > correct? So this would be the right conceptual level - and it would also 
> > remove the somewhat redundant 'kernel' part.
> 
> Well, all the MCE events reported are some way or the other tied to the
> CPU and they're reported in the CPU's MCA banks so I think we want
> 
> 	arch/x86/cpu/mce/

Well, *everything* the kernel does is in some way connected to a CPU, 
because we always execute the kernel on a CPU, still we have things like 
discrete PMUs and abstractions for other pieces of hardware that are not 
per CPU.

So the real question is, is there a signifcant class of MCE events that 
are not tied to the reporting channel which is per CPU (-ish ...) MCA 
banks?

Thanks,

	Ingo
Luck, Tony - Dec. 6, 2018, 5:36 p.m.
> So the real question is, is there a signifcant class of MCE events that 
> are not tied to the reporting channel which is per CPU (-ish ...) MCA 
> banks?

Perhaps QPI/UPI interconnect errors?

-Tony
Borislav Petkov - Dec. 6, 2018, 6:05 p.m.
On Thu, Dec 06, 2018 at 05:36:03PM +0000, Luck, Tony wrote:
> > So the real question is, is there a signifcant class of MCE events that 
> > are not tied to the reporting channel which is per CPU (-ish ...) MCA 
> > banks?
> 
> Perhaps QPI/UPI interconnect errors?

Whatever, this is pure bikeshedding what we're doing right now. All the
errors are reported through the CPU's MCA banks and the argument whether
an error class which is maybe not really tied to the CPU, should dictate
which path to put the files in, does not make any sense to me.

The only argument which makes sense IMO is whether the path should be
short. :-)

And both

arch/x86/cpu/mce/
arch/x86/mce/

are short enough to me.

Now, the real question is, how do we want to place the rest of the
files/folders so that we can get rid of deeper directory structures.

If we say, we don't want to have .../cpu/... and put everything in
arch/x86/ then this answers the above question too.

IMO, of course.

Patch

diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 1f5d2291c31e..43afe707c6fb 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -40,7 +40,7 @@  obj-$(CONFIG_INTEL_RDT)	+= intel_rdt.o intel_rdt_rdtgroup.o intel_rdt_monitor.o
 obj-$(CONFIG_INTEL_RDT)	+= intel_rdt_ctrlmondata.o intel_rdt_pseudo_lock.o
 CFLAGS_intel_rdt_pseudo_lock.o = -I$(src)
 
-obj-$(CONFIG_X86_MCE)			+= mcheck/
+obj-$(CONFIG_X86_MCE)			+= mce/
 obj-$(CONFIG_MTRR)			+= mtrr/
 obj-$(CONFIG_MICROCODE)			+= microcode/
 
diff --git a/arch/x86/kernel/cpu/mcheck/Makefile b/arch/x86/kernel/cpu/mce/Makefile
similarity index 52%
rename from arch/x86/kernel/cpu/mcheck/Makefile
rename to arch/x86/kernel/cpu/mce/Makefile
index bcc7c54c7041..765759765ab7 100644
--- a/arch/x86/kernel/cpu/mcheck/Makefile
+++ b/arch/x86/kernel/cpu/mce/Makefile
@@ -1,14 +1,14 @@ 
 # SPDX-License-Identifier: GPL-2.0
-obj-y				=  mce.o mce-severity.o mce-genpool.o
+obj-y				=  core.o severity.o genpool.o
 
 obj-$(CONFIG_X86_ANCIENT_MCE)	+= winchip.o p5.o
-obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
-obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
+obj-$(CONFIG_X86_MCE_INTEL)	+= intel.o
+obj-$(CONFIG_X86_MCE_AMD)	+= amd.o
 obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
-obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o
+obj-$(CONFIG_X86_MCE_INJECT)	+= inject.o
 
 obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
 
-obj-$(CONFIG_ACPI_APEI)		+= mce-apei.o
+obj-$(CONFIG_ACPI_APEI)		+= apei.o
 
 obj-$(CONFIG_X86_MCELOG_LEGACY)	+= dev-mcelog.o
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mce/amd.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce_amd.c
rename to arch/x86/kernel/cpu/mce/amd.c
index e12454e21b8a..4a2fb59a372e 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -28,7 +28,7 @@ 
 #include <asm/msr.h>
 #include <asm/trace/irq_vectors.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 #define NR_BLOCKS         5
 #define THRESHOLD_MAX     0xFFF
diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mce/apei.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce-apei.c
rename to arch/x86/kernel/cpu/mce/apei.c
index 2eee85379689..1d9b3ce662a0 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
+++ b/arch/x86/kernel/cpu/mce/apei.c
@@ -36,7 +36,7 @@ 
 #include <acpi/ghes.h>
 #include <asm/mce.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
 {
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mce/core.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce.c
rename to arch/x86/kernel/cpu/mce/core.c
index 36d2696c9563..b0ae12cf7827 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -52,7 +52,7 @@ 
 #include <asm/msr.h>
 #include <asm/reboot.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 static DEFINE_MUTEX(mce_log_mutex);
 
diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mce/dev-mcelog.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/dev-mcelog.c
rename to arch/x86/kernel/cpu/mce/dev-mcelog.c
index 27f394ac983f..41d9169d27fa 100644
--- a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
+++ b/arch/x86/kernel/cpu/mce/dev-mcelog.c
@@ -15,7 +15,7 @@ 
 #include <linux/kmod.h>
 #include <linux/poll.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 static BLOCKING_NOTIFIER_HEAD(mce_injector_chain);
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce-genpool.c b/arch/x86/kernel/cpu/mce/genpool.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce-genpool.c
rename to arch/x86/kernel/cpu/mce/genpool.c
index 217cd4449bc9..3395549c51d3 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-genpool.c
+++ b/arch/x86/kernel/cpu/mce/genpool.c
@@ -10,7 +10,7 @@ 
 #include <linux/mm.h>
 #include <linux/genalloc.h>
 #include <linux/llist.h>
-#include "mce-internal.h"
+#include "internal.h"
 
 /*
  * printk() is not safe in MCE context. This is a lock-less memory allocator
diff --git a/arch/x86/kernel/cpu/mcheck/mce-inject.c b/arch/x86/kernel/cpu/mce/inject.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce-inject.c
rename to arch/x86/kernel/cpu/mce/inject.c
index 1fc424c40a31..8492ef7d9015 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -38,7 +38,7 @@ 
 #include <asm/nmi.h>
 #include <asm/smp.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 /*
  * Collect all the MCi_XXX settings
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mce/intel.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce_intel.c
rename to arch/x86/kernel/cpu/mce/intel.c
index d05be307d081..e43eb6732630 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -18,7 +18,7 @@ 
 #include <asm/msr.h>
 #include <asm/mce.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 /*
  * Support for Intel Correct Machine Check Interrupts. This allows
diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mce/internal.h
similarity index 100%
rename from arch/x86/kernel/cpu/mcheck/mce-internal.h
rename to arch/x86/kernel/cpu/mce/internal.h
diff --git a/arch/x86/kernel/cpu/mcheck/p5.c b/arch/x86/kernel/cpu/mce/p5.c
similarity index 100%
rename from arch/x86/kernel/cpu/mcheck/p5.c
rename to arch/x86/kernel/cpu/mce/p5.c
diff --git a/arch/x86/kernel/cpu/mcheck/mce-severity.c b/arch/x86/kernel/cpu/mce/severity.c
similarity index 99%
rename from arch/x86/kernel/cpu/mcheck/mce-severity.c
rename to arch/x86/kernel/cpu/mce/severity.c
index 44396d521987..dc3e26e905a3 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-severity.c
+++ b/arch/x86/kernel/cpu/mce/severity.c
@@ -16,7 +16,7 @@ 
 #include <asm/mce.h>
 #include <linux/uaccess.h>
 
-#include "mce-internal.h"
+#include "internal.h"
 
 /*
  * Grade an mce by severity. In general the most severe ones are processed
diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mce/therm_throt.c
similarity index 100%
rename from arch/x86/kernel/cpu/mcheck/therm_throt.c
rename to arch/x86/kernel/cpu/mce/therm_throt.c
diff --git a/arch/x86/kernel/cpu/mcheck/threshold.c b/arch/x86/kernel/cpu/mce/threshold.c
similarity index 100%
rename from arch/x86/kernel/cpu/mcheck/threshold.c
rename to arch/x86/kernel/cpu/mce/threshold.c
diff --git a/arch/x86/kernel/cpu/mcheck/winchip.c b/arch/x86/kernel/cpu/mce/winchip.c
similarity index 100%
rename from arch/x86/kernel/cpu/mcheck/winchip.c
rename to arch/x86/kernel/cpu/mce/winchip.c