From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933871AbbDOQxy (ORCPT ); Wed, 15 Apr 2015 12:53:54 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:59034 "EHLO e36.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932583AbbDOQxp (ORCPT ); Wed, 15 Apr 2015 12:53:45 -0400 Message-ID: <552E9792.8010709@linux.vnet.ibm.com> Date: Wed, 15 Apr 2015 11:53:38 -0500 From: "Suresh E. Warrier" User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Peter Zijlstra CC: lklml , Steven Rostedt , Frederic Weisbecker , Ingo Molnar , Paul Mackerras Subject: Re: [PATCH 2/2] More precise timestamps for nested writes References: <1428459449-10664-1-git-send-email-warrier@linux.vnet.ibm.com> <552C7B77.1000607@linux.vnet.ibm.com> <552C7C0D.1090201@linux.vnet.ibm.com> <552C7D89.2080802@linux.vnet.ibm.com> <20150415091626.GF17717@twins.programming.kicks-ass.net> In-Reply-To: <20150415091626.GF17717@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15041516-0021-0000-0000-000009DBEA5C Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/15/2015 04:16 AM, Peter Zijlstra wrote: > On Mon, Apr 13, 2015 at 09:38:01PM -0500, Suresh E. Warrier wrote: > >> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c >> index c9b3005..0a2d862 100644 >> --- a/kernel/trace/ring_buffer.c >> +++ b/kernel/trace/ring_buffer.c >> @@ -482,6 +482,7 @@ struct ring_buffer_per_cpu { >> unsigned long read_bytes; >> u64 write_stamp; >> u64 read_stamp; >> + u64 last_stamp; >> /* ring buffer pages to update, > 0 to add, < 0 to remove */ >> int nr_pages_to_update; >> struct list_head new_pages; /* new pages to add */ > > So what is wrong with something like: > > u64 rb_get_delta(struct ring_buffer_per_cpu *cpu_buffer) > { > u64 last, now; > > for (;;) { > last = cpu_buffer->last_stamp; > now = cpu_buffer->buffer->clock(); > > if (cmpxchg_local(&cpu_buffer->last_stamp, last, now) == last) > break; > > cpu_relax(); > } > > return now - last; > } > > Of course, LL/SC on power stinks, but on most archs this is actually > fairly fast. > I am not sure it will help. The delta we assign to the current event has to be the difference of the "actual" timestamps of the last event in the buffer and the current event. If we can't be sure of the delta, we can let it be zero which is the current behavior for nested writes. Thus, one way to fix my patch to handle NMIs is to recheck the last_stamp after the event has been reserved in the buffer. If it has changed since we computed the delta, we can simply reset the delta to zero. This should only happen if an NMI occurs within the short critical section and it also does a trace. -suresh