Messages in this thread | | | Date | Thu, 27 Nov 2008 19:51:59 +0100 | From | "stephane eranian" <> | Subject | Re: [patch 02/24] perfmon: base code |
| |
thomas,
On Thu, Nov 27, 2008 at 7:28 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > Stephane, > > On Thu, 27 Nov 2008, stephane eranian wrote: >> > 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. > Good point.
>> >> + * -EBUSY: if conflicting session exist >> > >> > Where ? >> >> 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.
>> > 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. >> > >> 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?
>> > 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. >> >> > 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.
>> 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.
>> 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.
> 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.
>> 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.
> 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.
| |