* Reorganize perf kernel side
@ 2015-12-04 21:17 Borislav Petkov
2015-12-04 22:09 ` Peter Zijlstra
2015-12-06 9:54 ` Ingo Molnar
0 siblings, 2 replies; 4+ messages in thread
From: Borislav Petkov @ 2015-12-04 21:17 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: lkml
Hi guys,
so I've had my eyes on this for a long time now and it has managed to
get on my nerves just enough to do something about it :-)
So how about moving perf stuff to arch/x86/perf/ and get rid of the
prefixes in the filenames. This also flattens our folder structure which
is a good thing and which we've been talking about in the past.
In order to diminish churn, I can do the whole thing in 4-5 patches'
sets, after having run enough *config smoke tests and 0day bot too.
Anyway, something like that.
perf_event_<vendor>_<type>.c
can then move to arch/x86/perf/<vendor>/type.c
and have much saner structure.
Thoughts?
---
arch/x86/Kbuild | 2 ++
arch/x86/kernel/cpu/Makefile | 2 --
arch/x86/perf/Makefile | 1 +
arch/x86/{kernel/cpu => perf}/perf_event.c | 2 +-
4 files changed, 4 insertions(+), 3 deletions(-)
create mode 100644 arch/x86/perf/Makefile
rename arch/x86/{kernel/cpu => perf}/perf_event.c (99%)
diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
index 1538562cc720..cd85aa30ad14 100644
--- a/arch/x86/Kbuild
+++ b/arch/x86/Kbuild
@@ -1,6 +1,8 @@
obj-y += entry/
+obj-$(CONFIG_PERF_EVENTS) += perf/
+
obj-$(CONFIG_KVM) += kvm/
# Xen paravirtualization support
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 58031303e304..77000d54fcd1 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -30,8 +30,6 @@ obj-$(CONFIG_CPU_SUP_CENTAUR) += centaur.o
obj-$(CONFIG_CPU_SUP_TRANSMETA_32) += transmeta.o
obj-$(CONFIG_CPU_SUP_UMC_32) += umc.o
-obj-$(CONFIG_PERF_EVENTS) += perf_event.o
-
ifdef CONFIG_PERF_EVENTS
obj-$(CONFIG_CPU_SUP_AMD) += perf_event_amd.o perf_event_amd_uncore.o
ifdef CONFIG_AMD_IOMMU
diff --git a/arch/x86/perf/Makefile b/arch/x86/perf/Makefile
new file mode 100644
index 000000000000..3b2566aa58e5
--- /dev/null
+++ b/arch/x86/perf/Makefile
@@ -0,0 +1 @@
+obj-y += perf_event.o
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/perf/perf_event.c
similarity index 99%
rename from arch/x86/kernel/cpu/perf_event.c
rename to arch/x86/perf/perf_event.c
index 9dfbba5ce6e8..ccbf7242307c 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/perf/perf_event.c
@@ -37,7 +37,7 @@
#include <asm/desc.h>
#include <asm/ldt.h>
-#include "perf_event.h"
+#include "../kernel/cpu/perf_event.h"
struct x86_pmu x86_pmu __read_mostly;
--
2.3.5
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: Reorganize perf kernel side
2015-12-04 21:17 Reorganize perf kernel side Borislav Petkov
@ 2015-12-04 22:09 ` Peter Zijlstra
2015-12-04 22:40 ` Borislav Petkov
2015-12-06 9:54 ` Ingo Molnar
1 sibling, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2015-12-04 22:09 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Ingo Molnar, lkml
On Fri, Dec 04, 2015 at 10:17:56PM +0100, Borislav Petkov wrote:
> Hi guys,
>
> so I've had my eyes on this for a long time now and it has managed to
> get on my nerves just enough to do something about it :-)
>
> So how about moving perf stuff to arch/x86/perf/ and get rid of the
> prefixes in the filenames. This also flattens our folder structure which
> is a good thing and which we've been talking about in the past.
>
> In order to diminish churn, I can do the whole thing in 4-5 patches'
> sets, after having run enough *config smoke tests and 0day bot too.
> Anyway, something like that.
>
> perf_event_<vendor>_<type>.c
>
> can then move to arch/x86/perf/<vendor>/type.c
>
> and have much saner structure.
>
> Thoughts?
I _will_ blame you for at least a month after every time I mistype a
pathname because of this ;-)
git blame --follow must keep working. That is, git had better be able to
understand this code movement, loosing history is just a total PITA.
Also, a script that can auto-convert patches would be nice.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Reorganize perf kernel side
2015-12-04 22:09 ` Peter Zijlstra
@ 2015-12-04 22:40 ` Borislav Petkov
0 siblings, 0 replies; 4+ messages in thread
From: Borislav Petkov @ 2015-12-04 22:40 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, lkml
On Fri, Dec 04, 2015 at 11:09:07PM +0100, Peter Zijlstra wrote:
> I _will_ blame you for at least a month after every time I mistype a
> pathname because of this ;-)
Oh boy, I probably shouldn't do it after all out of fear of you coming
over ... :-)
> git blame --follow must keep working. That is, git had better be able to
> understand this code movement, loosing history is just a total PITA.
Well, I did
git annotate arch/x86/kernel/cpu/perf_event.c > blame.before
git annotate arch/x86/perf/perf_event.c > blame.after
The diff is:
--- /tmp/blame.before 2015-12-04 23:30:28.502701279 +0100
+++ /tmp/blame.after 2015-12-04 23:29:53.618702295 +0100
@@ -37,7 +37,7 @@ e3f3541c19c89 (Peter Zijlstra 2011-11-21
d07bdfd322d30 (Peter Zijlstra 2012-07-10 09:42:15 +0200 37)#include <asm/desc.h>
d07bdfd322d30 (Peter Zijlstra 2012-07-10 09:42:15 +0200 38)#include <asm/ldt.h>
241771ef016b5 (Ingo Molnar 2008-12-03 10:39:53 +0100 39)
-de0428a7ad485 (Kevin Winchester 2011-08-30 20:41:05 -0300 40)#include "perf_event.h"
+b5e0c1bab8637 (Borislav Petkov 2015-12-04 22:02:51 +0100 40)#include "../kernel/cpu/perf_event.h"
de0428a7ad485 (Kevin Winchester 2011-08-30 20:41:05 -0300 41)
de0428a7ad485 (Kevin Winchester 2011-08-30 20:41:05 -0300 42)struct x86_pmu x86_pmu __read_mostly;
efc9f05df2dd1 (Stephane Eranian 2011-06-06 16:57:03 +0200 43)
and that include file change is only temporary in order to keep the
churn at the lowest level.
Is this what you had in mind?
> Also, a script that can auto-convert patches would be nice.
Sure, I can do that.
Also, I was thinking of doing this in 4-5 patches sets only so that we
can keep everything easily manageable.
For sure, I don't want to do one insane move in one go and cause
needless churn and regressions.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Reorganize perf kernel side
2015-12-04 21:17 Reorganize perf kernel side Borislav Petkov
2015-12-04 22:09 ` Peter Zijlstra
@ 2015-12-06 9:54 ` Ingo Molnar
1 sibling, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2015-12-06 9:54 UTC (permalink / raw)
To: Borislav Petkov
Cc: Peter Zijlstra, lkml, Thomas Gleixner, Arnaldo Carvalho de Melo,
Jiri Olsa, Namhyung Kim
* Borislav Petkov <bp@alien8.de> wrote:
> Hi guys,
>
> so I've had my eyes on this for a long time now and it has managed to
> get on my nerves just enough to do something about it :-)
>
> So how about moving perf stuff to arch/x86/perf/ and get rid of the
> prefixes in the filenames. This also flattens our folder structure which
> is a good thing and which we've been talking about in the past.
>
> In order to diminish churn, I can do the whole thing in 4-5 patches'
> sets, after having run enough *config smoke tests and 0day bot too.
> Anyway, something like that.
>
> perf_event_<vendor>_<type>.c
>
> can then move to arch/x86/perf/<vendor>/type.c
>
> and have much saner structure.
>
> Thoughts?
Yeah, it would be lovely if you could do that - but could we please name it
'events' instead of 'perf', to follow the existing namespace pattern we are using
for the core bits, where we have kernel/events/ for the core bits, not
kernel/perf/?
Also, how about naming the core x86 bits like this:
arch/x86/events/core.c
which would give us a clear path to split-out core functionality eventually, such
as:
arch/x86/events/sched.c
arch/x86/events/constraints.c
...etc...
...
Just like we've already split out functionality from kernel/events/core.c into
kernel/events/ring_buffer.c.
Btw., kernel/sched/ is using a similar approach, there's core.c, and various
split-out sub-modules. We are slowly migrating from the original humunguous
kernel/sched.c to a more finegrained kernel/sched/subsys.c structure.
... and as you suggested, the x86 vendor dependent bits would be in their own, but
easily accessible directory close to the core, as you suggested:
arch/x86/events/intel/
arch/x86/events/amd/
...
as a lot of work is happening in that space, so promoting it up in the namespace
helps.
So, as an example, we'd have renames like this:
arch/x86/kernel/cpu/perf_event_intel_uncore_nhmex.c
=> arch/x86/events/intel/uncore_nhmex.c
which will be 30% easier to type! Once our muscle memory has re-trained that is. ;-)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-06 9:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-04 21:17 Reorganize perf kernel side Borislav Petkov
2015-12-04 22:09 ` Peter Zijlstra
2015-12-04 22:40 ` Borislav Petkov
2015-12-06 9:54 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).