From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathieu Desnoyers Subject: Re: [PATCH] vfs: Add a trace point in the mark_inode_dirty function Date: Mon, 29 Nov 2010 09:41:37 -0500 Message-ID: <20101129144137.GA10213@Krystal> References: <20101129103845.82A5.A69D9226@jp.fujitsu.com> <20101128205414.78384a8b@infradead.org> <20101129141513.82BA.A69D9226@jp.fujitsu.com> <1291040469.30543.715.camel@gandalf.stny.rr.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: KOSAKI Motohiro , Arjan van de Ven , Christoph Hellwig , linux-fsdevel@vger.kernel.org, Al Viro , Ingo Molnar , Peter Zijlstra To: Steven Rostedt Return-path: Received: from mail.openrapids.net ([64.15.138.104]:59059 "EHLO blackscsi.openrapids.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752452Ab0K2Oll (ORCPT ); Mon, 29 Nov 2010 09:41:41 -0500 Content-Disposition: inline In-Reply-To: <1291040469.30543.715.camel@gandalf.stny.rr.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: * Steven Rostedt (rostedt@goodmis.org) wrote: > [ Adding Ingo, Peter and Mathieu ] > > On Mon, 2010-11-29 at 14:15 +0900, KOSAKI Motohiro wrote: > > > On Mon, 29 Nov 2010 10:41:51 +0900 (JST) > > > KOSAKI Motohiro wrote: > > > > > > > > Signed-of-by: Arjan van de Ven > > > > > --- > > > > > fs/fs-writeback.c | 3 +++ > > > > > include/linux/fs.h | 12 ++++++++++++ > > > > > include/trace/events/writeback.h | 28 > > > > > ++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 0 > > > > > deletions(-) > > > > > > > > > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > > > > > index 3d06ccc..62e33cc 100644 > > > > > --- a/fs/fs-writeback.c > > > > > +++ b/fs/fs-writeback.c > > > > > @@ -952,6 +952,9 @@ void __mark_inode_dirty(struct inode *inode, > > > > > int flags) if ((inode->i_state & flags) == flags) > > > > > return; > > > > > > > > > > + if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC | > > > > > I_DIRTY_PAGES)) > > > > > + trace_writeback_inode_dirty(inode, flags); > > > > > + > > > > > > > > Why can't we move this branch into TP_fast_assign()? > > > > > > not really because then the tracepoint is already in process of being > > > emitted... no way to retract it anymore. > > > > I'm not tracing expert. but Steven said we can it in past. (cc him) > > > > http://www.spinics.net/lists/linux-ext4/msg20045.html > > > > Yes, this came up before. Currently, if you add the if statement in the > trcacepoint, you just wasted space. But if we can add a discard_entry, > or better yet, I can add a TRACE_EVENT_CONDITION() that adds an if > statement to check. Something like: > > TRACE_EVENT_CONDITION(name, > [...] > TP_condition( > if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC | > I_DIRTY_PAGES)) > return 1; > else > return 0; > ), Yep, something like this would be really useful for us too in user-space. One example use-case is to let a telecom application filter by "call ID" when following one call across multiple telecom switches. The kind of condition we would have is: TRACE_EVENT_CONDITION(name, [...] TP_condition( if (unlikely(call_id == monitored_call_id)) return 1; if (unlikely(!call_id_monitoring)) return 1; return 0; ), So in this case, "call_id" is received as parameter, but "monitored_call_id" and "call_id_monitoring" are global variables. One question that strikes me is: would you declare this outside of the TRACE_EVENT() or inside it ? How would you match the TRACE_EVENT() and the TRACE_EVENT_CONDITION() if they are separate, by name ? The dynamic "pre-filters" will also be useful, but I think this kind of statically defined conditions will answer a large amount of our requirements, letting these filtering expressions be designed by application developers and used by operators/engineers. Thanks, Mathieu > > Have this run on the parameters and not the entry fields (because the > entry fields are from the ring buffer, and to use them, we must first > write to the ring buffer (something we want to avoid). > > We could also make this code work with "pre-filters". That is, filters > on the tracepoints that access the parameters and not the final entries. > This would let us circumvent allocating ring buffer space when the > filter states to skip the entry. > > -- Steve > > > > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com