Hi Steven, On Fri, Feb 19, 2021 at 4:26 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > On Fri, 19 Feb 2021 07:53:53 +0200 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote: > > > Added new API for opening trace_marker file of given instance: > > tracefs_trace_marker_get_fd(); > > > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx> > > --- > > > > As I wrote the perf-trace.c program, I was thinking what we really should > have is the following API. We can keep this API, but what would be nice is: > > > int tracefs_print_init(struct tracefs_instance *instance); > > int tracefs_print(struct tracefs_instance *instance, > const char *fmt, ...); > > int tracefs_vprint(struct tracefs_instance *instance, > const char *fmt, va_list ap); > > void tracefs_print_reset(struct tracefs_instance *instance); > > Where tracefs_print_init() will open the trace_marker for that instance > (NULL being the top level), and storing it in the instance structure. You mean to hold the marker fd in the tracefs_instance structure ? I like such approach, to hold some library specific context in that structure, internally and not visible from the user. In that case we do not need tracefs_print_init() at all, the first call to some tracefs_print API will open the file. But that will make the APIs not thread safe, is it OK marker fd to be used from multiple threads at the same time ? > > tracefs_print() and tracefs_vprint() will check if the trace_marker file > has already been opened (tracefs_print_init() was previously called), and > if not, it will open it and keep it open. Then it will write to the > trace_marker file the passed in print data after formatting it (see my > trace_print in perf-trace.c). > > The tracefs_print_reset() will simply close the trace_marker file if it was > previously opened, note, so will any of the destructors of the instance. > > We could also have: > > int tracefs_raw_print_init(struct tracefs_instance *instance); > > int tracefs_raw_print(struct tracefs_instance *instance, > void *data, int len); > > void tracefs_raw_print_reset(struct tracefs_instance *instance); > > > That is the same, but instead of writing string data to the trace_marker, > it would write in memory into trace_marker_raw. I'm afraid that having too many APIs with sort of overlapping functionality could make the library hard to use ? Actually the proposed new API by this patch, tracefs_trace_marker_get_fd(), already duplicates the existing tracefs_instance_file_open() API. > > -- Steve -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center
|