* Steven Rostedt (rostedt@xxxxxxxxxxx) 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 <kosaki.motohiro@xxxxxxxxxxxxxx> wrote: > > > > > > > > Signed-of-by: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx> > > > > > --- > > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
