Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
On Thu, Dec 06, 2018 at 05:36:03PM +, 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. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
On Thu, Dec 06, 2018 at 05:36:03PM +, 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. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
RE: [PATCH] x86/mce: Streamline MCE subsystem's naming
> 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
RE: [PATCH] x86/mce: Streamline MCE subsystem's naming
> 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
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
* Borislav Petkov 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
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
* Borislav Petkov 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
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
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/ -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
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/ -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
* Borislav Petkov 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
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
* Borislav Petkov 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
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
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.gb29...@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 Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
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.gb29...@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 Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
On Wed, Dec 05, 2018 at 03:13:23PM +0100, Borislav Petkov wrote: > From: Borislav Petkov > > 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 Acked-by: Tony Luck
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
On Wed, Dec 05, 2018 at 03:13:23PM +0100, Borislav Petkov wrote: > From: Borislav Petkov > > 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 Acked-by: Tony Luck
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
* Borislav Petkov wrote: > From: Borislav Petkov > > 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 Thanks, Ingo
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
* Borislav Petkov wrote: > From: Borislav Petkov > > 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 Thanks, Ingo
