linux-kernel - Re: Tracing of power:power

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 12 May 2010 10:57:26 +0200
From:	Robert Schöne <robert.schoene@...dresden.de>
To:	Arjan van de Ven <arjan@...ux.intel.com>
Cc:	rostedt@...dmis.org,
	Ronny Tschüter <Ronny.Tschueter@...dresden.de>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Tracing of power:power_start events doesn't work

Am Mittwoch, den 05.05.2010, 07:33 -0700 schrieb Arjan van de Ven:
> On 5/5/2010 7:31, Steven Rostedt wrote:
> > On Wed, 2010-05-05 at 16:11 +0200, Ronny Tsch�ter wrote:
> >
> >>    /*
> >>     * Include the apic definitions for x86 to have the APIC timer related
> >> defines
> >> @@ -796,6 +797,18 @@ static int acpi_idle_bm_check(void)
> >>     */
> >>    static inline void acpi_idle_do_entry(struct acpi_processor_cx *cx)
> >>    {
> >> +    switch (cx->type) {
> >> +    case ACPI_STATE_C1:
> >> +        trace_power_start(POWER_CSTATE, 1);
> >> +        break;
> >> +    case ACPI_STATE_C2:
> >> +        trace_power_start(POWER_CSTATE, 2);
> >> +        break;
> >> +    case ACPI_STATE_C3:
> >> +        trace_power_start(POWER_CSTATE, 3);
> >> +        break;
> >> +    }
> >> +
> >
> > Depending on gcc, the above can bloat the code since each call to
> > trace_power_start() is a macro expanded. Try to call it just once.
> > Perhaps one of the following:
> 
> the code is also incorrect fundamentally.
> You need to pass in the mwait value or equivalent; the ACPI STATE type is
> pretty much useless random garbage and should completely be ignored.

You're correct for your case (switching to c-state via monitor/mwait).
But the current implementation is broken too. It works only if your
kernel uses processor_idle AND monitor/mwait. For all other cases it
fails - may it be on Ronnys system, that uses IO port based C-states or
my system, that uses cpu_idle (not processor_idle).

I'd suggest, that it should work for IO port based switches and could be
included in the syntactic block of that case.
Afaik, halt means that the processor goes to C1. So in this block, there
could be a power_start event too with 1 as new state. (Maybe there are
problems with AMDs C1E which would be reported as C1.)

Robert



> --
> 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/
> 


-- 
Robert Schoene
Technische Universitaet Dresden
Zentrum fuer Informationsdienste und Hochleistungsrechnen
01062 Dresden

Tel.: (0351) 463-42483, Fax: (0351) 463-37773
E-Mail: Robert.Schoene@...dresden.de

--
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/