linux-kernel - Re: [BUG(?)] perf_events: combining multiple tracepoint events into a group produces no counts on member events

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]

Message-ID: <20101201180237.GB3438@nowhere>
Date:	Wed, 1 Dec 2010 19:02:40 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Corey Ashford <cjashfor@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>,
	Stephane Eranian <eranian@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [BUG(?)] perf_events: combining multiple tracepoint events
 into a group produces no counts on member events

On Wed, Dec 01, 2010 at 01:04:37PM +0100, Peter Zijlstra wrote:
> @@ -545,6 +546,11 @@ struct hw_perf_event {
>  			struct task_struct		*bp_target;
>  		};
>  #endif
> +		/*
> +		 * Same fudge as for breakpoints, trace-events needs
> +		 * it too,.. convert the bp crap over..
> +		 */
> +		struct task_struct *event_target;

Yeah, looks like we can merge the bp_target and event_target.


>  struct task_struct {
>  	volatile long state;	/* -1 unrunnable, 0 runnable, >0 stopped */
>  	void *stack;
> @@ -1452,6 +1458,9 @@ struct task_struct {
>  	struct perf_event_context *perf_event_ctxp[perf_nr_task_contexts];
>  	struct mutex perf_event_mutex;
>  	struct list_head perf_event_list;
> +#ifdef CONFIG_EVENT_TRACING
> +	struct perf_tp_idr *perf_tp_idr;

Why not attaching this to the ctx eventually? This makes one pointer less
in task_struct.

> @@ -370,6 +372,7 @@ list_del_event(struct perf_event *event,
>  	 */
>  	if (event->state > PERF_EVENT_STATE_OFF)
>  		event->state = PERF_EVENT_STATE_OFF;
> +	++ctx->generation;

What's the role of the ctx->generation? It seems to be incremented two times
but doesn't appear to have any purpose.


>  }
>  
>  static void perf_group_detach(struct perf_event *event)
> @@ -1228,6 +1231,12 @@ void perf_event_context_sched_out(struct
>  	if (!cpuctx->task_ctx)
>  		return;
>  
> +#if 0
> +	/*
> +	 * Need to sort out how to make task_struct::perf_tp_idr
> +	 * work with this fancy switching stuff.. tracepoints could be
> +	 * in multiple contexts due to the software event muck.
> +	 */

Not sure what's the issue here. Each ctx have the perf_tp_idr matching
active tracepoints, isn't it?

> +static struct perf_tp_idr *perf_event_idr(struct perf_event *event, bool create)
> +{
> +	struct perf_tp_idr *tp_idr;
> +	struct task_struct *task;
> +
> +	if (event->attach_state & PERF_ATTACH_TASK) {
> +		task = event->hw.event_target;
> +		tp_idr = task->perf_tp_idr;
> +		if (!tp_idr && create)

Is it possible that task->perf_tp_idr can eventually disappear
under us there? Like when an event is released from that task?

> +			tp_idr = perf_tp_init_task(event, task);
> +	} else
> +		tp_idr = &per_cpu(perf_tp_idr, event->cpu);
> +
> +	return tp_idr;
> +}
--
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/