* Steven Rostedt (rostedt@xxxxxxxxxxx) wrote: > On Wed, 2009-12-02 at 13:06 -0500, Mathieu Desnoyers wrote: > > * > > Hrm. I wonder if having DEFINE_EVENT_CLASS is really worth having, > > considering that it really just does 2 things at once and may be > > confusing. > > We keep it because that's what TRACE_EVENT currently is. It would suck > to have to replace every TRACE_EVENT there is now with a > DECLARE_EVENT_CLASS and DEFINE_EVENT. Although this would push > developers into using classes. I agree that keeping something for backward compatibility is good, but what I dislike the most is the similarity between the DECLARE_EVENT_CLASS and DEFINE_EVENT_CLASS which have completely unrelated semantics. This is really misleading. > > > > > I would have thought amongst the lines of the following as main API > > (note: "SKETCH" is only a proposal. The idea is to do _not_ use > > declare/define, as it's really something _different_ than what people > > are expecting!) > > > > SKETCH_EVENT_CLASS() > > > > SKETCH_EVENT() > > > > Which would use only DECLARE, or both DECLARE and DEFINE depending if > > CREATE_TRACE_POINTS is set. I see the DECLARE/DEFINE more as the > > "low-level" macros that are actually selected by CREATE_TRACE_POINTS: > > > > DECLARE_EVENT_CLASS : only performs event class declarations (macros, > > inlines...) > > > > DECLARE_EVENT : only performs event instance declarations (macros, > > inlines, ...). Depends on the DECLARE_EVENT_CLASS(). > > > > DEFINE_EVENT_CLASS : create instances of template functions. > > > > DEFINE_EVENT : create event tracepoint functions. Depends on > > DEFINE_EVENT_CLASS(). > > > > This way, it should make digging into the generation system internals > > headhache-free. ;) I think we should really avoid re-using terms people > > are familiar with for things that have a semantic intrincially different > > than what people come to expect. > > Egad No! It would make it a living nightmare. The internals reuse the > define macro, and there's no intermediate. By changing the > DECLARE_EVENT_CLASS to another name (SKETCH_EVENT_CLASS) we would have > to add something like this: > > #define SKETCH_EVENT_CLASS(name, proto, args, tstruct, print) \ > DECLARE_EVENT_CLASS(name, PARAMS(proto), PARAMS(args),\ > PARAMS(tstruct), PARAMS(print)) > > We don't have a intermediate or "low level" macro in use here. Whatever > we give to the user is what we use. > Maybe we should consider having one. e.g.: #ifdef CREATE_TRACE_POINTS SKETCH_EVENT_CLASS maps to DEFINE_EVENT_CLASS #else SKETCH_EVENT_CLASS maps to DECLARE_EVENT_CLASS #endif > > I think the kernel developers are smart enough to figure out that these > macros are not a typical DECLARE/DEFINE that is elsewhere. But I think > using the DECLARE/DEFINE names will give them a better idea of what is > happening than to make up something completely new. In my opinion, re-using a well-known keyword (e.g. DECLARE/DEFINE) but applying a different semantic to what is generally agreed upon is a recipe for confusing developers and users, who will skip the review of some pieces of code assuming they already know what "DECLARE" and "DEFINE" stands for. I argue here that the content of trace/events/ headers are _not_ per se declarations nor definitions, and hence they should not confuse people by using inappropriately well-known keywords. They are actually more evolved macros that can be turned in either a declaration or definition, depending if CREATE_TRACE_POINTS is declared. When I created the markers/tracepoints, Andrew Morton explained to me the importance of distinguishing DECLARE vs DEFINE macros. I would really like to hear his point of view on the current question. Thanks, Mathieu > > -- Steve > -- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68 -- To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html