linux-kernel - Re: [RFC PATCH 1/4] perf: Starter and stopper 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]

Date:	Wed, 16 Mar 2011 15:21:11 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Lin Ming <minggr@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Paul Mackerras <paulus@...ba.org>,
	Stephane Eranian <eranian@...gle.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Hitoshi Mitake <mitake@....info.waseda.ac.jp>
Subject: Re: [RFC PATCH 1/4] perf: Starter and stopper events

On Tue, Mar 15, 2011 at 06:54:19PM +0100, Frederic Weisbecker wrote:
> On Tue, Mar 15, 2011 at 10:36:18PM +0800, Lin Ming wrote:
> > On Tue, Mar 15, 2011 at 3:18 AM, Frederic Weisbecker <fweisbec@...il.com> wrote:
> > 
> > >
> > > +static void perf_event_pause_resume(struct perf_event *event, int nmi,
> > > +                                   struct perf_sample_data *data,
> > > +                                   struct pt_regs *regs)
> > > +{
> > > +       struct perf_event *iter;
> > > +       unsigned long flags;
> > > +
> > > +       /*
> > > +        * Ensure the targets can't be sched in/out concurrently.
> > > +        * Disabling irqs is sufficient for that because starters/stoppers
> > > +        * are on the same cpu/task.
> > > +        */
> > > +       local_irq_save(flags);
> > 
> > Could you explain this more detail?
> 
> Yeah, I should have detailed that more.
> 
> So, I put a constraint in starters and stoppers: those must be attached
> to the same task and cpu than the target. That allows us to do this
> pause/resume lockless if we can ensure that:
> 
> - target sched in/out can't interrupt perf_event_pause_resume()
> - perf_event_pause_resume() can interrupt the target in the middle of
> event_sched_in()
> 
> So that both are strictly serialized.
> 
> We need to ensure that the target event can not be concurrently scheduled
> in (->add()) or scheduled out (->del() ), so that when we check
> PERF_EVENT_STATE_ACTIVE, we know that the event is currently running
> and is not going to move while we do our checks and we call start() and
> stop().
> 
> So the rationale is that the target can not be in the middle of
> event_sched_in() or event_sched_out() when the starter/stopper
> trigger. We have no guarantee of that currently, especially because
> of events that trigger in NMIs, but also for other corner cases may
> be, so I'll need to think about it later. Why not by using pmu_disable_all()
> on the starter/stopper when the target is about to schedule in/out, until
> we know the event->state really reflects the hardware and logical states.
> 
> Now event_sched_in() and event_sched_out() can still be called from an
> IPI to enable/disable an event. Hence the interrupts disabled to prevent
> from that.
>  
> > > +
> > > +
> > > +       /* Prevent the targets from being removed under us. */
> > > +       rcu_read_lock();

And BTW this rcu_read_lock() is not necessary. The target can not be removed
under us.

And also there is another race to take care about: if the starter and the stopper
trigger at the same time, we are going to call ->start() and ->stop() concurrently.
Not sure yet how to solve that.
--
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/