Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes: > On Sat, Mar 30, 2013 at 02:46:20AM +0200, Alexander Shishkin wrote: >> As part of the legacy from the original driver design, we retain home-grown >> tracing infrastructure, complete with own ring buffer and timestamps. While >> it is useful for debugging certain cases, it's a lot of extra code, which >> these days is rather redundant. >> >> This patch replaces local tracing functionality with kernel tracepoints, >> thus getting rid of the ring buffer and all the maintenance code, while >> making use of standard kernel infrastructure. > > What do these tracepoints do? Why do you still need them at all? These are quite useful for debugging the udc part of the driver. You can see especially the control traffic without having to use an analyser, with timestamps and whatnot. With perf, you can annotate certain (or all) tracepoints of interest with a call stack, which helps you see, for example, what the gadget driver was doing at that point. Without these, it will sometimes take you (me, more likely) a few cycles of "instrument code with debug prints -- grep through tons of crap in dmesg" before I can figure out a problem. With perf, I don't need to recompile or even reboot anything, it's all there. > Once you make a tracepoint, you are saying you are going to keep that > api for forever, are you really willing to do that? I'm not, which is What is wrong with that? This usb controller is not going to grow more registers or data structures in the future. I was even going to suggest a more generic set of tracepoints for all gadgets to use if they want. In fact, most of these tracepoints are applicable to any gadget right away, and will always be, since the structure of control traffic in usb 2.0 is not very likely to change. > why you don't see any USB tracepoints yet, and I really don't think that > a single driver needs them either. Well, if you do a "perf list -e tracepoint", you do get to see quite a few drivers. Not that I'm using this as a justification or anything. > So I need a whole lot of convincing before I can take a patch like this, > sorry. Tracepoints and perf provide incredibly powerful and useful means of debugging, especially when it comes to things like usb, when you are interested not only in the events per se, but also in their order and timing. Same thing goes for profiling. All that said, I'm now going to change the patchset to just remove the old event buffer churn. Since nobody has really shown interest in these tracepoints, folks are probably happy with printk debugging, which is also fine by me, because I'm not the one who has to do most of the debugging in the chipidea driver. The old event buffer is what's *bothering* me. > I took the first 3 patches here, but odds are, if I can't take this one, > the rest will not apply, right? Of course. Regards, -- Alex -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
