On Tue, Jul 2, 2013 at 9:09 AM, Adrian Hunter <adrian.hun...@intel.com> wrote: > On 01/07/13 21:53, David Ahern wrote: >> On 7/1/13 3:32 AM, Adrian Hunter wrote: >>> Snip >>> >>>> >>>> While this works for a combined S/W and tracepoint events session, I do not >>>> like promoting sample types to the minimum compatible level for all events >>>> in the session. perf needs to allow each event to have its own sample_type >>>> and not force a minimal compatibility. >>> >>> Why? The impact is small. The kernel API is completely unchanged. >> >> I'd like to see libperf become a stable, usable library - usable by more >> than the perf binary and its builtin commands. I have already done this once >> for a daemon, and it was a PITA to get the specific use functional without >> memory leaks/growth in the libperf part. >> >> With respect to this specific patch it means appropriate flexibility in the >> data collected for events. ie., each event can have its own sample_type. For >> example if the tracepoint already contains task information TID is not >> needed - and IP may not be wanted either. The code processing the samples >> should not require all events to have some minimum data format - that just >> wastes buffer space. > > It would be more compelling to provide a use-case where that "waste" > actually makes any difference. > In my opinion, it's not so much of the "wasted" space than on the ease of use for tools. With your change, tools would have to know something about the order in which sample_type is laid out. And it just happens that it is not as trivial as expected. It is NOT the bit position order in sample_type. So this is more error prone.
I prefer your IDENTIFIER solution better, yet it also implies that this flag is special. It provides the event ID at the first position in the sample's body. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
