linux-kernel - Re: [RFC PATCH 4/4] perf/core: Schedule every pinned events before the the non-pinned

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] [day] [month] [year] [list]

Message-ID: <20091110103411.GD5255@nowhere>
Date:	Tue, 10 Nov 2009 11:34:13 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Mike Galbraith <efault@....de>,
	Paul Mackerras <paulus@...ba.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC PATCH 4/4] perf/core: Schedule every pinned events before
	the the non-pinned

On Tue, Nov 10, 2009 at 11:10:13AM +0100, Peter Zijlstra wrote:
> On Sun, 2009-11-08 at 21:13 +0100, Frederic Weisbecker wrote:
> 
> > +static void
> > +__perf_event_sched_in_all(struct perf_event_context *ctx,
> > +			  struct perf_cpu_context *cpuctx, int cpu)
> > +{
> > +	struct perf_event_context *cpu_ctx = &cpuctx->ctx;
> > +
> > +	/* May require different classes between cpu and task lock */
> > +	spin_lock(&cpu_ctx->lock);
> > +	spin_lock(&ctx->lock);
> 
> Would be good to know for sure, running with lockdep enabled ought to
> tell you that pretty quick ;-)



That's about sure I guess :)
I just wanted to take care of that after your comments.


 
> > +	cpu_ctx->is_active = ctx->is_active = 1;
> > +
> > +	ctx->timestamp = cpu_ctx->timestamp = perf_clock();
> > +
> > +	perf_disable();
> > +
> > +	if (cpu_ctx->nr_events)
> > +		__perf_event_sched_in_pinned(cpu_ctx, cpuctx, cpu);
> > +
> > +	if (ctx->nr_events)
> > +		__perf_event_sched_in_pinned(cpu_ctx, cpuctx, cpu);
> > +
> > +	if (cpu_ctx->nr_events)
> > +		__perf_event_sched_in_volatile(cpu_ctx, cpuctx, cpu);
> > +
> > +	if (ctx->nr_events)
> > +		__perf_event_sched_in_volatile(cpu_ctx, cpuctx, cpu);
> > +
> > +	cpuctx->task_ctx = ctx;
> > +
> > +	perf_enable();
> > +
> > +	spin_unlock(&ctx->lock);
> > +	spin_lock(&cpu_ctx->lock);
> 
> I'm pretty sure that ought to be spin_unlock() ;-)


Indeed :)


> > +}
> 
> 
> Like Ingo I don't really like the volatile name.
> 
> Can't we simply have 2 lists per cpu a pinned and normal list, and first
> schedule all the pinned and RR the normal events?
> 
> I guess one could implement that by adding the task context events to
> the cpu context events on sched_in and removing them on sched_out. That
> would clear up a lot of funny scheduling details.


I thought about doing that, but didn't expand the idea that much,
because of the list manipulation that induces.

But you're right, that would be be indeed more proper.
I can just save the "real" cpu event group tail in the
struct perf_cpu_context so that I can keep track of the real
state and (un)glue the queues easily.

Yeah, I'll try that, thanks!

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