Message-ID: <20090827011718.GA5315@localhost.localdomain>
Date:	Wed, 26 Aug 2009 21:17:18 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Ingo Molnar <mingo@...e.hu>, David Miller <davem@...emloft.net>,
	fweisbec@...il.com, billfink@...dspring.com,
	netdev@...r.kernel.org, brice@...i.com, gallatin@...i.com
Subject: Re: Receive side performance issue with multi-10-GigE and NUMA

On Wed, Aug 26, 2009 at 08:29:59PM -0400, Steven Rostedt wrote:
> 
> On Wed, 26 Aug 2009, Neil Horman wrote:
> > > 
> > I'm not sure how the addition of an ftrace module constitutes a change to the
> > tracing infrastructure, but whatever, yes, no biggy.  I've bugun modifying the
> > TRACE_EVENT that I added to export the data I need directly.  Should be pretty
> > straightforward.  Dave I'll have a patch up on netdev in a day or two after I
> > test it.  Steven, should this still just go to netdev with a cc to you?  I'd
> > like to avoid repeating the same confusion here a second time around if I can
> 
> Yes, please Cc myself, and Ingo on those changes. I see where the 
> confusion came. It is where the code changes. The code in kernel/trace is 
> considered ftrace internals (there's internal tracing upkeep that is 
> needed for all plugins). But with TRACE_EVENT, those can happen totally 
> inside a subsystem without touching any tracing directory. Those are 
> yours, and the TRACE_EVENT is just an API to the rest of the kernel. We 
> don't even care if you add a header to include/trace/events/ (if it 
> follows the standard format).
> 
> But by adding a plugin, it causes more work for us. The plugin types do 
> not get automated like TRACE_EVENTs and for binary readers like perf and 
> trace-cmd, we need to hand export the binary format for them.
> 
Understood, I'll keep that in mind in the future.

> > 
> > > > 
> > > > Ok, I'm rather tired of arguing.  Dave, I'll leave this in your hands.  The code
> > > > I wrote works fairly well in my view, and I feel like the review on it was both
> > > > positive and sufficent for inclusion.  But thats not my call, its yours.  I can
> > > > meet my own need with a raw TRACE_EVENT for now just as easily.  IF you feel
> > > > like the skb plugin should be pulled, please do so, and let me know.  All I ask
> > > > is that you keep the skb_copy_datagram_iovec TRACE_EVENT in place.  If you pull
> > > > the ftrace plugin, I'll submit a subsequent patch to agument the printing format
> > > > so that I can gather the numa allocation and consumption data directly there.
> > > 
> > > Yes, please keep the TRACE_EVENT (I think we can all agree on that ;-).
> > > 
> > Yes, that is rather centeral to what I'm monitoring :)
> > 
> > > You probably already read my previous email on the matter. Don't delete 
> > > your plugin patch until we get everything you need with TRACE_EVENT alone.
> > > 
> > Its ok, I should have the TRACE_EVENT modified to export this stuff directly by
> > tomorrow or friday anyway.  I really honestly just liked the ftrace interface
> > better, I found it a bit less confusing :)
> 
> Heh, because it was just a bit of cut and paste. But as Frederic said, 
> very much prone to errors. And it breaks the binary userspace readers. 
> Your new type did not get exported via trace_export.c.
> 
> TRACE_EVENT can be a little harder to learn, because it is all MACRO 
> magic, but once you understand them, you'll find that they are very easy.
> 
Yeah, macro magic is an understatement.  But I'll have the conversions done in
the next few days, no worries.

Thanks!
Neil

> Thanks!
> 
> -- Steve
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists