Re: [PATCH 2/2] More precise timestamps for nested writes

Peter Zijlstra
2015-04-15T08:46:10Z

On Tue, Apr 14, 2015 at 07:33:00PM -0500, Suresh E. Warrier wrote:
> On 04/14/2015 12:13 PM, Peter Zijlstra wrote:
> > On Mon, Apr 13, 2015 at 09:38:01PM -0500, Suresh E. Warrier wrote:
> >> +static u64 *get_write_timestamp(struct ring_buffer_per_cpu *cpu_buffer,
> >> + unsigned long *flags)
> >> +{
> >> + if (rb_precise_nested_write_ts()) {
> >> + /*
> >> + * Ensure that we are not preempted until after we update
> >> + * the write timestamp.
> >> + */
> >> + local_irq_save(*flags);
> >> + return &cpu_buffer->last_stamp;
> >
> > Yeah, ever hear about NMIs? This isn't going to work.
>
> That is a good point! If a NMI can come in and start running a handler
> that can generate a trace event, this code is indeed broken.
>
> Some architectures like PowerPC don't have NMIs like Intel and so
> I hadn't thought of that. Thanks for catching that!

Learn you arch better, Power actually has the nesting level.
local_irq_disable() is a software disable, only if an actual interrupt
comes in while that flag is set do you disable the irq in hardware.

Now Paul Mackerras (ab)used this to allow PMU interrupts while soft
disabled, which effectively gets you NMI nesting on Power.