On 12/10/2013 09:53 PM, Mauro Carvalho Chehab wrote: > Em Sat, 23 Nov 2013 17:54:48 +0100 > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > >> On 11/23/2013 05:30 PM, Sylwester Nawrocki wrote: >>> Hi, >>> >>> On 11/23/2013 12:25 PM, Hans Verkuil wrote: >>>> Hi Wade, >>>> >>>> On 11/22/2013 08:48 PM, Wade Farnsworth wrote: >>>>> Add tracepoints to the QBUF and DQBUF ioctls to enable rudimentary >>>>> performance measurements using standard kernel tracers. >>>>> >>>>> Signed-off-by: Wade Farnsworth<wade_farnsworth@xxxxxxxxxx> >>>>> --- >>>>> >>>>> This is the update to the RFC patch I posted a few weeks back. I've added >>>>> several bits of metadata to the tracepoint output per Mauro's suggestion. >>>> >>>> I don't like this. All v4l2 ioctls can already be traced by doing e.g. >>>> echo 1 (or echo 2)>/sys/class/video4linux/video0/debug. >>>> >>>> So this code basically duplicates that functionality. It would be nice to be able >>>> to tie in the existing tracing code (v4l2-ioctl.c) into tracepoints. >>> >>> I think it would be really nice to have this kind of support for standard >>> traces at the v4l2 subsystem. Presumably it could even gradually replace >>> the v4l2 custom debug infrastructure. >>> >>> If I understand things correctly, the current tracing/profiling >>> infrastructure >>> is much less invasive than inserting printks all over, which may cause >>> changes >>> in control flow. I doubt the system could be reliably profiled by >>> enabling all >>> those debug prints. >>> >>> So my vote would be to add support for standard tracers, like in other >>> subsystems in the kernel. >> >> The reason for the current system is to trace which ioctls are called in >> what order by a misbehaving application. It's very useful for that, >> especially when trying to debug user problems. >> >> I don't mind switching to tracepoints as long as this functionality is >> kept one way or another. > > I agree with Sylwester: we should move to tracepoints, and this is a good > start. As I mentioned, I don't mind switching to tracepoints, but not in the way the current patch does it. I certainly don't agree with you merging this patch as-is without further discussion. To make it official: Nacked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> If we do tracepoints, then we do it right and for all ioctls, not in this half-baked manner. Please revert. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
