linux-kernel - Re: [tip:perf/core] perf: Rework the PMU methods

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:	Mon, 13 Sep 2010 14:15:53 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Michael Cree <mcree@...on.net.nz>
Cc:	mingo@...hat.com, dengcheng.zhu@...il.com,
	yanmin_zhang@...ux.intel.com, gorcunov@...il.com,
	fweisbec@...il.com, robert.richter@....com, ming.m.lin@...el.com,
	tglx@...utronix.de, hpa@...or.com, paulus@...ba.org,
	linux-kernel@...r.kernel.org, eranian@...glemail.com,
	will.deacon@....com, lethal@...ux-sh.org, davem@...emloft.net,
	mingo@...e.hu, linux-alpha@...r.kernel.org
Subject: Re: [tip:perf/core] perf: Rework the PMU methods

On Sun, 2010-09-12 at 17:33 +1200, Michael Cree wrote:

> Yes, done.  I also took the liberty to fix an undefined variable and 
> multiple defined variable errors that were exposed by compilation.  Will 
> reply to this with the patch.

Thanks, and sorry for messing up Alpha that bad.. I have an alpha
compiler and I really through I compile tested it :/

> I've also tested it on a UP alpha.  It worked well for a little while 
> but after running 'perf top' for a number of seconds I got the following 
> warning:

<snip warn>

> which is from the line in alpha_pmu_start() that checks that 
> PERF_HES_STOPPED is set.
> 
> I see that the backtrace is from the Alpha timer_interrupt() code which 
> goes something like this:
> 
> [do some stuff updating timer deltas then...]
> 
> #ifndef CONFIG_SMP
>          while (nticks--)
>                  update_process_times(user_mode(get_irq_regs()));
> #endif
> 
>          if (test_perf_event_pending()) {
>                  clear_perf_event_pending();
>                  perf_event_do_pending();
>          }
> 
>          return IRQ_HANDLED;
> }
> 
> 
> When I added the code for handle pending events to the timer interrupt I 
> hadn't realised that update_process_times() could call back into the 
> perf code.  I'm speculating here, but could it be that there is pending 
> work to stop the HW counter, but the call to re-start it is beating the 
> call to stop it?

Right, so the ->start() call came from perf_ctx_adjust_freq(), which
depending on whether perf_adjust_period() gets inlined, can have two
such calls.

Assuming it didn't inline (there's two callsites, which should defeat
the inline static functions with a single callsite heuristic), you hit
the unthrottle() call.

Ahh, the alpha throttle call should be using the fancy new stop function
too (will fold into your earlier patch if it indeed works):

As to the point you raised above, yes, I think it would be prudent to
call perf_event_do_pending() before update_process_times().


Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>
---
Index: linux-2.6/arch/alpha/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/perf_event.c
+++ linux-2.6/arch/alpha/kernel/perf_event.c
@@ -825,14 +825,14 @@ static void alpha_perf_event_irq_handler
 			break;
 	}
 
+	event = cpuc->event[j];
+
 	if (unlikely(j == cpuc->n_events)) {
 		/* This can occur if the event is disabled right on a PMC overflow. */
-		wrperfmon(PERFMON_CMD_ENABLE, cpuc->idx_mask);
+		alpha_pmu_stop(event, 0);
 		return;
 	}
 
-	event = cpuc->event[j];
-
 	if (unlikely(!event)) {
 		/* This should never occur! */
 		irq_err_count++;

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