Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1306147 > unrolled thread

Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1 Ingo Molnar <mingo@kernel.org> - 2016-01-11 13:40 +0100
    Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1 Borislav Petkov <bp@alien8.de> - 2016-01-11 15:40 +0100
      Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1 Ingo Molnar <mingo@kernel.org> - 2016-01-11 16:20 +0100
        Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1 Borislav Petkov <bp@alien8.de> - 2016-01-11 17:40 +0100
          Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1 Ingo Molnar <mingo@kernel.org> - 2016-01-12 12:20 +0100

#1306147 — Re: [RFC PATCH 0/5] perf: Sanitize perf directory structure, p1

* Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> Hi guys,
> 
> so what better time to do that fun than during the holidays when no one
> else is producing patches. :-)
> 
> Anyway, here's a part one conversion of the AMD perf events facilities.
> Directory structure looks like this:
> 
> arch/x86/events/
> |-- amd
> |   |-- core.c
> |   |-- ibs.c
> |   |-- iommu.c
> |   |-- iommu.h
> |   `-- uncore.c
> |-- core.c
> `-- Makefile
> 
> and I've kept the churn at a mininum. Holler if something's not kosher.

So it is only now that I noticed that we have both 'core.c' and 'uncore.c', which 
is slightly bit confusing: the core.c's we have around the kernel denote core 
subsystem functionality - not CPU core functionality. If we have uncore.c around, 
confusion might ensue.

OTOH maybe it's not a big issue and we can just live with it. I'd hate to see 
main.c. Maybe rename 'uncore.c' to 'cpu_uncore.c'? OTOH that sounds a bit silly 
too. So maybe leave it as you named them.

Thanks,

	Ingo

[toc] | [next] | [standalone]

#1306319

On Mon, Jan 11, 2016 at 01:39:44PM +0100, Ingo Molnar wrote:
> So it is only now that I noticed that we have both 'core.c' and 'uncore.c', which 
> is slightly bit confusing: the core.c's we have around the kernel denote core 
> subsystem functionality - not CPU core functionality. If we have uncore.c around, 
> confusion might ensue.
> 
> OTOH maybe it's not a big issue and we can just live with it. I'd hate to see 
> main.c. Maybe rename 'uncore.c' to 'cpu_uncore.c'? OTOH that sounds a bit silly 
> too. So maybe leave it as you named them.

Well, not doung anything should be ok because the *uncore* things would
go to the <vendor>/ subdir:

I.e.,

arch/x86/kernel/cpu/perf_event_amd_uncore.c
arch/x86/kernel/cpu/perf_event_intel_uncore.c
arch/x86/kernel/cpu/perf_event_intel_uncore.h
arch/x86/kernel/cpu/perf_event_intel_uncore_nhmex.c
arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c

will become

arch/x86/events/amd/uncore.c
arch/x86/events/intel/uncore.c
arch/x86/events/intel/uncore.h
arch/x86/events/intel/uncore_nhmex.c
arch/x86/events/intel/uncore_snb.c
arch/x86/events/intel/uncore_snbep.c

and core.c will be in the upper dir arch/x86/events/

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1306383

* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Jan 11, 2016 at 01:39:44PM +0100, Ingo Molnar wrote:
> > So it is only now that I noticed that we have both 'core.c' and 'uncore.c', which 
> > is slightly bit confusing: the core.c's we have around the kernel denote core 
> > subsystem functionality - not CPU core functionality. If we have uncore.c around, 
> > confusion might ensue.
> > 
> > OTOH maybe it's not a big issue and we can just live with it. I'd hate to see 
> > main.c. Maybe rename 'uncore.c' to 'cpu_uncore.c'? OTOH that sounds a bit silly 
> > too. So maybe leave it as you named them.
> 
> Well, not doung anything should be ok because the *uncore* things would
> go to the <vendor>/ subdir:
> 
> I.e.,
> 
> arch/x86/kernel/cpu/perf_event_amd_uncore.c
> arch/x86/kernel/cpu/perf_event_intel_uncore.c
> arch/x86/kernel/cpu/perf_event_intel_uncore.h
> arch/x86/kernel/cpu/perf_event_intel_uncore_nhmex.c
> arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c
> 
> will become
> 
> arch/x86/events/amd/uncore.c
> arch/x86/events/intel/uncore.c
> arch/x86/events/intel/uncore.h
> arch/x86/events/intel/uncore_nhmex.c
> arch/x86/events/intel/uncore_snb.c
> arch/x86/events/intel/uncore_snbep.c
> 
> and core.c will be in the upper dir arch/x86/events/

Ok, works for me. Got confused by your ascii art:

arch/x86/events/
|-- amd
|   |-- core.c
|   |-- ibs.c
|   |-- iommu.c
|   |-- iommu.h
|   `-- uncore.c
|-- core.c
`-- Makefile

which lists events/amd/core.c.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1306482

On Mon, Jan 11, 2016 at 04:14:12PM +0100, Ingo Molnar wrote:
> Ok, works for me. Got confused by your ascii art:
> 
> arch/x86/events/
> |-- amd
> |   |-- core.c
> |   |-- ibs.c
> |   |-- iommu.c
> |   |-- iommu.h
> |   `-- uncore.c
> |-- core.c
> `-- Makefile
> 
> which lists events/amd/core.c.

No, that's all correct. There is an amd/core.c thing:

 arch/x86/{kernel/cpu/perf_event_amd.c => events/amd/core.c}       | 2 +-

and there's also a arch/x86/kernel/cpu/perf_event_intel.c which would want to
become events/intel/core.c

Hmm, what do we call those then?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1307279

* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Jan 11, 2016 at 04:14:12PM +0100, Ingo Molnar wrote:
> > Ok, works for me. Got confused by your ascii art:
> > 
> > arch/x86/events/
> > |-- amd
> > |   |-- core.c
> > |   |-- ibs.c
> > |   |-- iommu.c
> > |   |-- iommu.h
> > |   `-- uncore.c
> > |-- core.c
> > `-- Makefile
> > 
> > which lists events/amd/core.c.
> 
> No, that's all correct. There is an amd/core.c thing:
> 
>  arch/x86/{kernel/cpu/perf_event_amd.c => events/amd/core.c}       | 2 +-
> 
> and there's also a arch/x86/kernel/cpu/perf_event_intel.c which would want to
> become events/intel/core.c
> 
> Hmm, what do we call those then?

core.c is fine with me!

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web