Messages in this thread | | | Date | Thu, 25 Sep 2008 13:15:59 -0400 | From | Mathieu Desnoyers <> | Subject | Re: [RFC PATCH 1/3] Unified trace buffer |
| |
* Linus Torvalds (torvalds@linux-foundation.org) wrote: > > > On Wed, 24 Sep 2008, Mathieu Desnoyers wrote: > > > > The reason why Martin did use only a 27 bits TSC in ktrace was that they > > were statically limited to 32 event types. > > Well, I actually think we could do the same - for the "internal" types. > > So why not do something like 4-5 bits for the basic type information, and > then oen of those cases is a "freeform" thing, and the others are reserved > for other uses. > > So a trace entry header could easily look something like > > struct trace_entry { > u32 tsc_delta:27, > type:5; > u32 data; > u64 array[]; > } > > and then depending on the that 5-bit type, the "data" field in the header > means different things, and the size of the trace_entry also is different. > > So it could be something like > > - case 0: EnfOfPage marker > (data is ignored) > size = 8 >
We could use a page header instead to contain the "unused_size" information. It does not need to be an event per se. Putting this information in the page header makes it easy for a consumer to just read the amount of bytes needed, excluding the padding (turns up to be useful for network streaming of trace data). Also, it frees up one event ID for other uses. I think the event ID real estate is pretty important, because every event ID we don't keep for "internal uses" could be used to encode standard tracer event IDs.
> - case 1: TSCExtend marker > data = extended TSC (bits 28..59) > size = 8 >
I would prefer to put the extended timestamp within the event header instead of creating a separate entry for this for atomicity concerns (what happens if a long interrupt executes between the TSCExtend marker event and the event expecting to be written right next to it ?). If we choose to have such "full tsc" event headers, we would have to reserve 1 selection bit. It leaves us 4 bits for event IDs. If we remove heartbeats, we need even less "internal" IDs.
Given that we can allocate event IDs per buffer, if in general we assume that buffer users will have few event IDs, I think we can find a way to minimize the number of "internal" event IDs and keep possibly all the 4 bits (16 IDs) for real tracer events. One way to achieve it is to encode the extra typing information within a table (dumped in a separate buffer) indexed by event ID.
> - case 2: TimeStamp marker > data = tv_nsec > array[0] = tv_sec > size = 16 >
This one could even be a standard event put in a single buffer. There is no need to repeat it in various buffers all over the place.
> - case 3: LargeBinaryBlob marker > data = 32-bit length of binary data > array[0] = 64-bit pointer to binary blob > array[1] = 64-bit pointer to "free" function > size = 24 > > - case 4: SmallBinaryBlob marker > data = inline length in bytes, must be < 4096 > array[0..(len+7)/8] = inline data, padded > size = (len+15) & ~7 > > - case 5: AsciiFormat marker > data = number of arguments > array[0] = 64-bit pointer to static const format string > array[1..arg] = argument values > size = 8*(2+arg) > > ... > > ie we use a few bits for "trace _internal_ type fields", and then for a > few of those types we have internal meanings, and other types just means > that the user can fill in the data itself. > > IOW, you _could_ have an interface like > > ascii_marker_2(ringbuffer, > "Reading sector %lu-%lu", > sector, sector+nsec); > > and what it would create would be a fairly small trace packet that looks > something like > > .type = 5, > .tsc_delta = ..., > .data = 2, > .array[0] = (const char *) "Reading sector %lu-%lu\n" > .array[1] = xx, > .array[2] = yy >
I agree that exporting semantic information is important. Moreover, I think this should also be made available when the trace is exported in binary format to userspace. The markers currently in mainline has been designed to do this efficiently. With small adaptation of the markers, one could do a :
trace_mark(block, read, "Reading sector sec_from to sec_to", "sec_from %lu sec_to %lu", sector, sector + nsec);
The nice part about the markers is that it keeps tables for the description "Reading sector sec_from to sec_to" and the typing information "sec_from %lu sec_to %lu" in a separate section. Therefore, we only need to dump this information once (at trace start or when the module containing this specific marker is loaded). It automatically deals with module load/unload and does not require to write their format string in the trace buffers.
The strings can be looked up by a userspace pretty-printer by dumping a table mapping event IDs to marker names, which in turn map to description and event types. This implies creating a small "metadata" buffer along with each data transfer buffer to export these tables. This metadata buffer must be read first to get the event typing information, and then the userspace program is all set to pretty-print the binary information.
An efficient ID -> format string mapping could also be kept around in the kernel (or built on demand) to simplify the task for an in-kernel pretty-printer.
Mathieu
> and you would not actually print it out as ASCII until somebody read it > from the kernel (and any "binary" interface would get the string as a > string, not as a pointer, because the pointer is obviously meaningless > outside the kernel. > > Also note how you'd literally just have a single copy of the string, > because the rule would be that a trace user must use a static string, not > some generated one that can go away (module unloading would need to be > aware of any trace buffer entries, of course - perhaps by just disallowing > unloading while trace buffers are active). > > And note! Everything above is meant as an example of something that > _could_ work. I do like the notion of putting pointers to strings in the > markers, rather than having some odd magic numerical meaning that user > space has to just magically know that "event type 56 for ring buffer type > 171 means that there are two words that mean 'sector' and 'end-sector' > respectively". > > But it's still meant more as an RFC. But I think it could work. > > Linus >
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |