perfmon: base code
PATCH 2/24 perfmon: base code
This patch adds the core functions of the perfmon subsystem. It creates the toplevel perfmon subdirectory where all perfmon generic code lives. Core functions implements initialization, session allocation and termination.
--
perfmon/perfmon_attach.c new file +86 -0
@@ -0,0 +1,86 @@ +/* +* perfmon_attach.c: perfmon2 load/unload functions +* +* This file implements the perfmon2 interface which +* provides access to the hardware performance counters +* of the host processor. +* +* +* The initial version of perfmon.c was written by +* Ganesh Venkitachalam, IBM Corp. +* +* Then it was modified for perfmon-1.x by Stephane Eranian and +* David Mosberger, Hewlett Packard Co. +* +* Version Perfmon-2.x is a complete rewrite of perfmon-1.x +* by Stephane Eranian, Hewlett Packard Co. +* +* Copyright (c) 1999-2006 Hewlett-Packard Development Company, L.P. +* Contributed by Stephane Eranian <[email protected]> +* David Mosberger-Tang <[email protected]> +* +* More information about perfmon available at: +* http://www.hpl.hp.com/research/linux/perfmon +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of version 2 of the GNU General Public +* License as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA +* 02111-1307 USA +*/ +#include <linux/kernel.h> +#include <linux/fs.h> +#include <linux/perfmon_kern.h> +#include "perfmon_priv.h" + +/** +* __pfm_exit_thread - detach and free context on thread exit +*/ +void __pfm_exit_thread (void ) +{ +struct pfm_context *ctx; +unsigned long flags; +int free_ok =0 , ret =-1 ; + + ctx = current->pfm_context; + +spin_lock_irqsave (&ctx->lock, flags); + +PFM_DBG ("state=%d is_self=%d" , ctx->state, ctx->flags.is_self); + +/* +* __pfm_unload_context() cannot fail +* in the context states we are interested in +*/ +switch (ctx->state) { +case PFM_CTX_LOADED : + ret =__pfm_unload_context (ctx); +break ; +case PFM_CTX_ZOMBIE : + ret =__pfm_unload_context (ctx); + free_ok =1 ; +break ; +default : +BUG_ON (ctx->state !=PFM_CTX_LOADED ); +break ; + } +spin_unlock_irqrestore (&ctx->lock, flags); + +if (!ret) +pfm_session_release (); + +/* +* All memory free operations (especially for vmalloc'ed memory) +* MUST be done with interrupts ENABLED. +*/ +if (free_ok) +pfm_free_context (ctx); +}
perfmon/perfmon_ctx.c new file +164 -0
164 lines not on this page · raw message
perfmon/perfmon_file.c new file +94 -0
94 lines not on this page · raw message
perfmon/perfmon_init.c new file +84 -0
84 lines not on this page · raw message
perfmon/perfmon_priv.h new file +110 -0
110 lines not on this page · raw message
perfmon/perfmon_res.c new file +105 -0
105 lines not on this page · raw message
include/linux/sched.h +3 -0
@@ -1352,6 +1352,9 @@unsigned long default_timer_slack_ns;struct list_head *scm_work_list; +#ifdef CONFIG_PERFMON +struct pfm_context *pfm_context; +#endif };/*
perfmon/perfmon_attach.c line 72
Quoted 13 lines · On Wed, Nov 26, 2008 at 12:42:00AM -0800, [email protected] wrote:
perfmon/perfmon_attach.c new file +13
-0+* __pfm_unload_context() cannot fail +* in the context states we are interested in +*/ +switch (ctx->state) { +case PFM_CTX_LOADED : + ret =__pfm_unload_context (ctx); +break ; +case PFM_CTX_ZOMBIE : + ret =__pfm_unload_context (ctx); + free_ok =1 ; +break ; +default : +BUG_ON (ctx->state !=PFM_CTX_LOADED );
That is just BUG() because the condition is always true here.
Quoted 4 lines
+ *
+ * we come here if:
+ *
+ * - we are zombie and we need to cleanup our state
...
perfmon/perfmon_ctx.c line 68
Quoted 8 lines
perfmon/perfmon_ctx.c new file +8
-0+void pfm_handle_work (struct pt_regs *regs) +{ +struct pfm_context *ctx; +unsigned long flags; +int type; + +if (!user_mode (regs)) +return ;
zombies are never in user mode. Either the comment is wrong or the test.
perfmon/perfmon_ctx.c line 123
Quoted 13 lines
perfmon/perfmon_ctx.c new file +13
-0+* enable interrupt for vfree() +*/ +local_irq_enable (); + +/* +* actual context free +*/ +pfm_free_context (ctx); + +/* +* restore interrupts as they were upon entry +*/ +local_irq_restore (flags);
That seems redundant after the local_irq_enable() above.
perfmon/perfmon_file.c line 46
Quoted 10 lines
+ */
perfmon/perfmon_file.c new file +9
-0+#include <linux/kernel.h> +#include <linux/fs.h> +#include <linux/file.h> +#include <linux/vfs.h> +#include <linux/mount.h> +#include <linux/perfmon_kern.h> +#include "perfmon_priv.h" + +#define PFMFS_MAGIC 0xa0b4d889/* perfmon filesystem magic number */
This should be probably somewhere else.
perfmon/perfmon_file.c line 59
Quoted 3 lines
perfmon/perfmon_file.c new file +3
-0+return 1 ; +} +__setup ("perfmon_debug" , enable_debug);
There's a new generic dynamic_printk.h now that might be an alternative.
Quoted 11 lines
perfmon/perfmon_init.c new file +11
-0+{ +PFM_LOG ("version %u.%u" ,PFM_VERSION_MAJ ,PFM_VERSION_MIN ); + +if (pfm_init_ctx ()) + goto error_disable; + +if (pfm_init_fs ()) + goto error_disable; + +/* +* one time, arch-specific global initialization
perfmon/perfmon_init.c line 76
Quoted 10 lines
perfmon/perfmon_init.c new file +10
-0+*/ +if (pfm_arch_init ()) + goto error_disable; + +return 0 ; + +error_disable: +PFM_ERR ("perfmon is disabled due to initialization error" ); + perfmon_disabled =1 ; +return -1 ;
I presume failure is very unlikely, but this has leaks under various error conditions, hasn't it?
If they are very unlikely a comment that you don't handle it intentionally is enough, but at least the comment if not proper code should be there.
perfmon/perfmon_priv.h line 83
Quoted 9 lines
perfmon/perfmon_priv.h new file +9
-0+{ +#ifdef CONFIG_SMP +struct pfm_context *ctxp; + + ctxp =__get_cpu_var (pmu_ctx); +if (!ctxp) +return ; +/* +* in UP per-thread, due to lazy save
But that's in CONFIG_SMP?
Also that assumption might be not true on !x86?
Quoted 11 lines
perfmon/perfmon_priv.h new file +11
-0+* there could be a context from another +* task. We need to push it first before +* installing our new state +*/ +pfm_save_pmds (ctxp); +/* +* do not clear ownership because we rewrite +* right away +*/ +#endif +}
perfmon/perfmon_priv.h
Quoted 10 lines
perfmon/perfmon_priv.h new file +10
-0+* global information about all sessions +*/ +struct pfm_resources { +cpumask_t sys_cpumask;/* bitmask of used cpus */ +u32 thread_sessions;/* #num loaded per-thread sessions */ +}; + +static struct pfm_resources pfm_res; + +static __cacheline_aligned_in_smp DEFINE_SPINLOCK (pfm_res_lock );
Would it make sense to share other global perfmon context on the same cache line?
perfmon/perfmon_res.c line 80
Quoted 13 lines
perfmon/perfmon_res.c new file +13
-0+*/ +spin_lock_irqsave (&pfm_res_lock, flags); + +PFM_DBG ("in thread=%u" , + pfm_res.thread_sessions); + + pfm_res.thread_sessions++; + +PFM_DBG ("out thread=%u ret=%d" , + pfm_res.thread_sessions, + ret); + +spin_unlock_irqrestore (&pfm_res_lock, flags);
The usual pattern would be to just do this with an atomic counter. I suppose it wouldn't buy scalability, but perhaps shorter code.
-Andi
Signature · 1 line
[email protected]
Stephane,
Quoted 6 lines · On Wed, 26 Nov 2008, [email protected] wrote:
+/*
+ * pfmfs should _never_ be mounted by userland - too much of security hassle,
+ * no real gain from having the whole whorehouse mounted. So we don't need
+ * any operations on the root directory. However, we need a non-trivial
+ *d_name- pfm: will go nicely and kill the special-casing in procfs.
+ */
Can we please use anon_inodefs instead of creating a new one ? It's exaclty for this purpose.
See fs/anon_inodes.c and fs/timerfd.c for an usage example.
Thanks,
tglx
perfmon/perfmon_res.c line 48
Quoted 9 lines · On Wed, 26 Nov 2008, [email protected] wrote:
Index:
o3/perfmon/perfmon_res.cperfmon/perfmon_res.c new file +7
-0+/* +* global information about all sessions +*/ +struct pfm_resources { +cpumask_t sys_cpumask;/* bitmask of used cpus */ +u32 thread_sessions;/* #num loaded per-thread sessions */ +};
What's the purpose of this being a structure if it's just a single instance ?
perfmon/perfmon_res.c line 59
Quoted 10 lines
perfmon/perfmon_res.c new file +9
-0+static struct pfm_resources pfm_res; + +static __cacheline_aligned_in_smp DEFINE_SPINLOCK (pfm_res_lock ); +/** +* pfm_session_acquire - reserve a per-thread session +* +* return: +* 0 : success +* -EBUSY: if conflicting session exist
Where ?
perfmon/perfmon_res.c line 68
Quoted 9 lines
+ */
perfmon/perfmon_res.c new file +8
-0+int pfm_session_acquire (void ) +{ +unsigned long flags; +int ret =0 ; + +/* +* validy checks on cpu_mask have been done upstream +*/
How please ? pfm_res.sys_cpumask is local to this file and you want to check it under the lock and _before_ you increment thread_sessions blindly.
Quoted 24 lines
perfmon/perfmon_res.c new file +24
-0+spin_lock_irqsave (&pfm_res_lock, flags); + +PFM_DBG ("in thread=%u" , + pfm_res.thread_sessions); + + pfm_res.thread_sessions++; + +PFM_DBG ("out thread=%u ret=%d" , + pfm_res.thread_sessions, + ret); + +spin_unlock_irqrestore (&pfm_res_lock, flags); + +return ret; +} + +/** +* pfm_session_release - release a per-thread session +* +* called from __pfm_unload_context() +*/ +void pfm_session_release (void ) +{ +unsigned long flags;
perfmon/perfmon_res.c line 102
Quoted 10 lines
perfmon/perfmon_res.c new file +10
-0+ +spin_lock_irqsave (&pfm_res_lock, flags); + +PFM_DBG ("in thread=%u" , + pfm_res.thread_sessions); + + pfm_res.thread_sessions--; + +PFM_DBG ("out thread=%u" , + pfm_res.thread_sessions);
What's the value of these debugs ? Prove that the compiler managed to compile "pfm_res.thread_sessions--;" correctly ?
A WARN_ON(!pfm_res.thread_sessions) instead of blindly decrementing would be way more useful.
Review
Quoted 2 lines
(quoted diff) +2
-0+ spin_unlock_irqrestore(&pfm_res_lock, flags); +}
<adding the bits from the oprofile patch, which belong here as they are not x86 specific>
+
+/**
+ * pfm_session_allcpus_acquire - acquire per-cpu sessions on all available cpus
+ *
+ * currently used by Oprofile on X86
+ */
+int pfm_session_allcpus_acquire(void)
+ for_each_online_cpu(cpu) {
+ cpu_set(cpu, pfm_res.sys_cpumask);
+ nsys_cpus++;
+ }
Sigh, why do we need a loop to copy a bitfield ?
+/**
+ * pfm_session_allcpus_release - relase per-cpu sessions on all cpus
+ *
+ * currently used by Oprofile code
+ */
+void pfm_session_allcpus_release(void)
+{
+ unsigned long flags;
+ u32 nsys_cpus, cpu;
+
+ spin_lock_irqsave(&pfm_res_lock, flags);
+
+ nsys_cpus = cpus_weight(pfm_res.sys_cpumask);
+
+ PFM_DBG("in sys=%u task=%u",
+ nsys_cpus,
+ pfm_res.thread_sessions);
+
+ /*
+ * XXX: could use __cpus_clear() with nbits
+ */
__cpus_clear(pfm_res.sys_cpumask, nsys_cpus); ????
That'd be real fun with a sparse mask.
+ for_each_online_cpu(cpu) {
+ cpu_clear(cpu, pfm_res.sys_cpumask);
+ nsys_cpus--;
+ }
Yuck. cpus_clear() perhaps ?
+EXPORT_SYMBOL(pfm_session_allcpus_release);
All what that code should do (in fact it does not) is preventing the mix of thread and system wide sessions.
It does neither need a cpumask nor tons of useless loops and debug outputs with zero value.
static int global_session;
static int thread_sessions;
static DEFINE_SPINLOCK(session_lock);
int pfm_session_request(int global)
{
unsigned long flags;
int res = -EBUSY;
spin_lock_irqsave(&session_lock, flags);
if (!global && !global_session) {
thread_sessions++;
res = 0;
}
if (global && !thread_sessions && !global_session) {
global_session = 1;
res = 0;
}spin_unlock_irqrestore(&session_lock, flags); return res; }
void pfm_session_release(int global)
{
unsigned long flags;
spin_lock_irqsave(&session_lock, flags);
if (global) {
WARN_ON(!global_session);
global_session = 0;
} else {
if (!global_session && thread_sessions)
thread_session--;
else
WARN();
}
spin_unlock_irqrestore(&session_lock, flags);
}Would do it nicely including useful sanity checks and 70% less code.
Thanks,
tglx
Thomas,
Quoted 4 lines · Thomas Gleixner
Quoted 1 line · On Wed, 26 Nov 2008, [email protected] wrote:
Index:
o3/perfmon/perfmon_res.c
Quoted 11 lines
perfmon/perfmon_res.c line 48
Quoted 7 lines
perfmon/perfmon_res.c new file +7
-0+/* +* global information about all sessions +*/ +struct pfm_resources { +cpumask_t sys_cpumask;/* bitmask of used cpus */ +u32 thread_sessions;/* #num loaded per-thread sessions */ +};What's the purpose of this being a structure if it's just a single instance ?
There is a single instance.
I was just trying to regroup related fields together instead of having them as separate variables. I can make the change.
Quoted 4 lines
Quoted 3 lines
perfmon/perfmon_res.c new file +3
-0+static struct pfm_resources pfm_res; + +static __cacheline_aligned_in_smp DEFINE_SPINLOCK (pfm_res_lock );
Quoted 8 lines
Quoted 6 lines
+/**
+ *pfm_session_acquire- reserve a per-thread session
+ *
+ * return:
+ * 0 : success
+ *-EBUSY: if conflicting session existWhere ?
Not in the patchset, conflict can arise when you add system-wide sessions.
Quoted 15 lines
perfmon/perfmon_res.c line 68
Quoted 10 lines
+ */
perfmon/perfmon_res.c new file +8
-0+int pfm_session_acquire (void ) +{ +unsigned long flags; +int ret =0 ; + +/* +* validy checks on cpu_mask have been done upstream +*/ How please ? pfm_res.sys_cpumask is local to this file and you want to check it under the lock and _before_ you increment
thread_sessionsblindly.
Stale comment.
Quoted 6 lines
+EXPORT_SYMBOL(pfm_session_allcpus_release);
All what that code should do (in fact it does not) is preventing the mix of thread and system wide sessions.
True. That is a current limitation.
Quoted 3 lines
It does neither need a cpumask nor tons of useless loops and debug outputs with zero value.
Well, the the cpumask is indeed needed but you don't see the reason why in the patchset!
Perfmon in system-wide does not operate like Oprofile. In system-wide a perfmon session (context) monitors only ONE CPU at a time. Each session is independent of each other. You can therefore measure different things on different CPUs. Reservation is thus done independently for each CPU, therefore we need a cpu bitmask to track allocation.
The Oprofile reservation you see is built on top of the cpumask reservation. It tries to allocate in one call and atomically ALL the CPUs as this is the way Oprofile operates. Thus it fails if one perfmon system-wide session or one perfmon per-thread exists. I believe there is enough bitmask functions to avoid loops as you pointed out. I will fix that.
Quoted 50 lines
static int
global_session;
static intthread_sessions;
staticDEFINE_SPINLOCK(session_lock);int
pfm_session_request(int global){ unsigned long flags; int res = -EBUSY; spin_lock_irqsave(&session_lock, flags); if (!global && !global_session) { thread_sessions++; res = 0; } if (global && !thread_sessions && !global_session) { global_session = 1; res = 0; } spin_unlock_irqrestore(&session_lock, flags); return res; }void
pfm_session_release(int global){ unsigned long flags; spin_lock_irqsave(&session_lock, flags); if (global) { WARN_ON(!global_session); global_session = 0; } else { if (!global_session && thread_sessions) thread_session--; else WARN(); } spin_unlock_irqrestore(&session_lock, flags); }Would do it nicely including useful sanity checks and 70% less code.
Thanks,
tglx
Stephane,
Quoted 6 lines · stephane eranian
Quoted 3 lines
What's the purpose of this being a structure if it's just a single instance ?
There is a single instance.
I was just trying to regroup related fields together instead of having them as separate variables. I can make the change.
Well, if you do a structure then put the lock in it as well, so its on one cacheline.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
+ *
-EBUSY: if conflicting session existWhere ?
Not in the patchset, conflict can arise when you add system-wide sessions.
Well, conflicts arise when oprofile is running as well, isn't it ?
Quoted 5 lines
Quoted 4 lines
How please ? pfm_res.sys_cpumask is local to this file and you want to check it under the lock and _before_ you increment
thread_sessionsblindly.Stale comment.
Well, where is it checked ? Where is checked whether Oprofile runs or not ?
Quoted 10 lines
Quoted 3 lines
All what that code should do (in fact it does not) is preventing the mix of thread and system wide sessions.
True. That is a current limitation.
Quoted 3 lines
It does neither need a cpumask nor tons of useless loops and debug outputs with zero value.
Well, the the cpumask is indeed needed but you don't see the reason why in the patchset!
If its not needed now, then we can either remove it or do at least something useful with it.
Quoted 2 lines
Perfmon in system-wide does not operate like Oprofile. In system-wide a perfmon session (context) monitors only ONE CPU at a time. Each
Then it is a percpu session and not system wide. Please use less confusing names.
Quoted 3 lines
session is independent of each other. You can therefore measure different things on different CPUs. Reservation is thus done independently for each CPU, therefore we need a cpu bitmask to track allocation.
Ok. Question: if you do a one CPU wide session with perfom, can you still do thread monitoring on the same CPU ?
If no, what prevents that a monitored thread is migrated to such a CPU ?
Quoted 4 lines
The Oprofile reservation you see is built on top of the cpumask reservation. It tries to allocate in one call and atomically ALL the CPUs as this is the way Oprofile operates. Thus it fails if one perfmon system-wide session or one perfmon per-thread exists.
This only prevents oprofile from starting, but it does neither prevent thread sessions nor does it prevent a perfmon per cpu session on a cpu which was onlined after oprofile started, simply because it's bit is missing in the CPU mask.
Oprofile if active starts profiling on cpu hotplug, but if you look at the cpumask with a perfmon per cpu request it will succeed.
If you do resource management and that is what the file claims to do, then you need to do it in a consistent way:
Oprofile can only run, when no thread and per cpu perfmon jobs are active.
Perfmon per cpu and thread jobs can only run when oprofile is not active.
Not sure about the thread vs. per cpu perfmon situation. See question above.
Thanks,
tglx
Quoted 1 line
Well, where is it checked ? Where is checked whether Oprofile runs or not ?
That is done using the perfctr reservation. I saw that somewhere in the patchkit. The NMI watchdog uses that too.
Quoted 9 lines
Quoted 4 lines
The Oprofile reservation you see is built on top of the cpumask reservation. It tries to allocate in one call and atomically ALL the CPUs as this is the way Oprofile operates. Thus it fails if one perfmon system-wide session or one perfmon per-thread exists.
This only prevents oprofile from starting, but it does neither prevent thread sessions nor does it prevent a perfmon per cpu session on a cpu which was onlined after oprofile started, simply because it's bit is missing in the CPU mask.
The perfctr reservation is global over all CPUs.
-Andi
Signature · 1 line
[email protected]
Quoted 10 lines · Andi Kleen
Quoted 1 line
Well, where is it checked ? Where is checked whether Oprofile runs or not ?
That is done using the perfctr reservation. I saw that somewhere in the patchkit. The NMI watchdog uses that too.
Quoted 5 lines
Quoted 4 lines
The Oprofile reservation you see is built on top of the cpumask reservation. It tries to allocate in one call and atomically ALL the CPUs as this is the way Oprofile operates. Thus it fails if one perfmon system-wide session or one perfmon per-thread exists.
Quoted 6 lines
Quoted 4 lines
This only prevents oprofile from starting, but it does neither prevent thread sessions nor does it prevent a perfmon per cpu session on a cpu which was onlined after oprofile started, simply because it's bit is missing in the CPU mask.
The perfctr reservation is global over all CPUs.
So this mean we manage resources on multiple levels, some bits here and some bits there and five checks in each code path do get them all.
Really convincing concept.
Thanks,
tglx
Thomas,
Quoted 17 lines · Thomas Gleixner
Quoted 15 lines · Andi Kleen
Quoted 1 line
Well, where is it checked ? Where is checked whether Oprofile runs or not ?
That is done using the perfctr reservation. I saw that somewhere in the patchkit. The NMI watchdog uses that too.
Quoted 9 lines
Quoted 4 lines
The Oprofile reservation you see is built on top of the cpumask reservation. It tries to allocate in one call and atomically ALL the CPUs as this is the way Oprofile operates. Thus it fails if one perfmon system-wide session or one perfmon per-thread exists.
This only prevents oprofile from starting, but it does neither prevent thread sessions nor does it prevent a perfmon per cpu session on a cpu which was onlined after oprofile started, simply because it's bit is missing in the CPU mask.
Quoted 6 lines
Quoted 1 line
The perfctr reservation is global over all CPUs.
So this mean we manage resources on multiple levels, some bits here and some bits there and five checks in each code path do get them all.
Really convincing concept.
That is a different level of resource management: register level.
Both Perfmon and Oprofile can operate with fewer registers than what the hardware actually supports. Take an AMD64 CPU with 4 counters. With the NMI watchdog active, Perfmon or Oprofile will run with only 3. That's better than nothing and is sufficient for many measurments.
thomas,
Quoted 6 lines · Thomas Gleixner
Stephane,
Quoted 3 lines · stephane eranian
Quoted 3 lines
What's the purpose of this being a structure if it's just a single instance ?
Quoted 7 lines
Quoted 3 lines
There is a single instance.
I was just trying to regroup related fields together instead of having them as separate variables. I can make the change.Well, if you do a structure then put the lock in it as well, so its on one cacheline.
Good point.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
+ *
-EBUSY: if conflicting session existWhere ?
Not in the patchset, conflict can arise when you add system-wide sessions.
Well, conflicts arise when oprofile is running as well, isn't it ?
Correct.
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
How please ? pfm_res.sys_cpumask is local to this file and you want to check it under the lock and _before_ you increment
thread_sessionsblindly.Stale comment.
Well, where is it checked ? Where is checked whether Oprofile runs or not ?
It does not care whether it is Oprofile, NMI or any other subsystem. What matters
is:
- what PMU registers are available?
- what CPU are not used for monitoring?
- are there per-thread sessions running?
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
All what that code should do (in fact it does not) is preventing the mix of thread and system wide sessions.
True. That is a current limitation.
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
It does neither need a cpumask nor tons of useless loops and debug outputs with zero value.
Well, the the cpumask is indeed needed but you don't see the reason why in the patchset!
If its not needed now, then we can either remove it or do at least something useful with it.
That something useful is the reserve all or nothing for Oprofile.
Quoted 6 lines
Quoted 2 lines
Perfmon in system-wide does not operate like Oprofile. In system-wide a perfmon session (context) monitors only ONE CPU at a time. Each
Then it is a percpu session and not system wide. Please use less confusing names.
I know that. I have used this name since the beginning, it's more legacy than anything else. Let's call this cpu-wide mode. I think people are more familiar with the notion of system-wide than cpu-wide.
Quoted 7 lines
Quoted 3 lines
session is independent of each other. You can therefore measure different things on different CPUs. Reservation is thus done independently for each CPU, therefore we need a cpu bitmask to track allocation.
Ok. Question: if you do a one CPU wide session with perfom, can you still do thread monitoring on the same CPU ?
No. They are currently mutually exclusive.
Quoted 2 lines
If no, what prevents that a monitored thread is migrated to such a CPU ?
Nothing. AND you don't want to change affinity because you are monitoring. So the current restriction is that cpu-wide and per-thread are mutually exclusive.
The only way to avoid that is to partition the PMU register so each can co-exist on the same CPU. I have not reached that point yet. They are also some hardware limitations which prevent that from being implemented, e.g., on Itanium.
Quoted 10 lines
Quoted 4 lines
The Oprofile reservation you see is built on top of the cpumask reservation. It tries to allocate in one call and atomically ALL the CPUs as this is the way Oprofile operates. Thus it fails if one perfmon system-wide session or one perfmon per-thread exists.
This only prevents oprofile from starting, but it does neither prevent thread sessions nor does it prevent a perfmon per cpu session on a cpu which was onlined after oprofile started, simply because it's bit is missing in the CPU mask.
That is a good point!
The test needs to be more sophisticated than that. I guess we can keep the 'global' variable you've introduced and check against that first, then check individual bits for conflicting perfmon cpu-wide session.
Quoted 11 lines
Oprofile if active starts profiling on cpu hotplug, but if you look at the cpumask with a perfmon per cpu request it will succeed.
If you do resource management and that is what the file claims to do, then you need to do it in a consistent way:
Oprofile can only run, when no thread and per cpu perfmon jobs are active.
Perfmon per cpu and thread jobs can only run when oprofile is not active.
Not sure about the thread vs. per cpu perfmon situation. See question above.
Stephane,
Quoted 7 lines · stephane eranian
Quoted 7 lines
Quoted 3 lines
session is independent of each other. You can therefore measure different things on different CPUs. Reservation is thus done independently for each CPU, therefore we need a cpu bitmask to track allocation.
Ok. Question: if you do a one CPU wide session with perfom, can you still do thread monitoring on the same CPU ?
Quoted 7 lines
No. They are currently mutually exclusive.
Quoted 2 lines
If no, what prevents that a monitored thread is migrated to such a CPU ?
Nothing. AND you don't want to change affinity because you are monitoring. So the current restriction is that cpu-wide and per-thread are mutually exclusive.
And how is this achieved ? Currently there seems nothing which prevents a per-thread vs. cpu-wide monitoring.
Thanks,
tglx
Quoted 13 lines · Thomas Gleixner
Stephane,
Quoted 9 lines · stephane eranian
Quoted 7 lines
Quoted 3 lines
session is independent of each other. You can therefore measure different things on different CPUs. Reservation is thus done independently for each CPU, therefore we need a cpu bitmask to track allocation.
Ok. Question: if you do a one CPU wide session with perfom, can you still do thread monitoring on the same CPU ?
No. They are currently mutually exclusive.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
If no, what prevents that a monitored thread is migrated to such a CPU ?
Nothing. AND you don't want to change affinity because you are monitoring. So the current restriction is that cpu-wide and per-thread are mutually exclusive.
And how is this achieved ? Currently there seems nothing which prevents a per-thread vs. cpu-wide monitoring.
That's true, but that's because cpu-wide support is not included in the patchset.
Quoted 20 lines · stephane eranian
Quoted 19 lines · Thomas Gleixner
Stephane,
Quoted 14 lines · stephane eranian
Quoted 7 lines
Quoted 3 lines
session is independent of each other. You can therefore measure different things on different CPUs. Reservation is thus done independently for each CPU, therefore we need a cpu bitmask to track allocation.
Ok. Question: if you do a one CPU wide session with perfom, can you still do thread monitoring on the same CPU ?
No. They are currently mutually exclusive.
Quoted 2 lines
If no, what prevents that a monitored thread is migrated to such a CPU ?
Nothing. AND you don't want to change affinity because you are monitoring. So the current restriction is that cpu-wide and per-thread are mutually exclusive.
Quoted 5 lines
Quoted 3 lines
And how is this achieved ? Currently there seems nothing which prevents a per-thread vs. cpu-wide monitoring.
That's true, but that's because cpu-wide support is not included in the patchset.
That's the whole point I'm making. For the current patch set the simple global vs. thread exclusivity is sufficient and correct.
When we gradually add stuff then we simply can add the extra checks and think about the impact and consequences in that context.
Thanks,
tglx