Message-ID: <7c86c4470811271051g66669197qb6b5fd39961668a9@mail.gmail.com>
Date:	Thu, 27 Nov 2008 19:51:59 +0100
From:	"stephane eranian" <eranian@...glemail.com>
To:	"Thomas Gleixner" <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	mingo@...e.hu, x86@...nel.org, andi@...stfloor.org,
	sfr@...b.auug.org.au
Subject: Re: [patch 02/24] perfmon: base code

thomas,

On Thu, Nov 27, 2008 at 7:28 PM, Thomas Gleixner <tglx@...utronix.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists