Re: [PATCH v1 1/3] libtracefs: Add user_events to libtracefs sources

[Steven Rostedt <rostedt@xxxxxxxxxxx>]

On Fri, 18 Feb 2022 14:50:56 -0800
Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> wrote:

> diff --git a/include/tracefs-local.h b/include/tracefs-local.h
> index bf157e1..e768cba 100644
> --- a/include/tracefs-local.h
> +++ b/include/tracefs-local.h
> @@ -119,4 +119,28 @@ int trace_rescan_events(struct tep_handle *tep,
>  struct tep_event *get_tep_event(struct tep_handle *tep,
>  				const char *system, const char *name);
>  
> +/* Internal interface for ftrace user events */
> +
> +struct tracefs_user_event_group;
> +
> +struct tracefs_user_event
> +{
> +	int write_index;
> +	int status_index;
> +	int iovecs;
> +	int rels;
> +	int len;
> +	struct tracefs_user_event_group *group;
> +	struct tracefs_user_event *next;
> +};
> +
> +struct tracefs_user_event_group
> +{
> +	int fd;
> +	int mmap_len;
> +	char *mmap;
> +	pthread_mutex_t lock;
> +	struct tracefs_user_event *events;
> +};
> +
>  #endif /* _TRACE_FS_LOCAL_H */
>



> +/**
> + * tracefs_user_event_test - Tests if an event is currently enabled
> + * @event: User event to test
> + *
> + * Tests if the @event is valid and currently enabled on the system.
> + *
> + * Return true if enabled, false otherwise.
> + */
> +bool tracefs_user_event_test(struct tracefs_user_event *event)
> +{
> +	return event && event->group->mmap[event->status_index] != 0;
> +}
> +

I was thinking we could even make the above faster by making it a static
inline in the tracefs.h header file.

In tracefs.h:

struct tracefs_user_event {
	char			*enable;
};

static inline bool tracefs_user_event_test(struct tracefs_user_event *event)
{
	return event && event->enable[0] != 0;
}

Then in tracefs-local.h:

struct tracefs_user_event_internal {
	struct tracefs_user_event	event_external;
	[...]
};

Then have in tracefs_user_event_register():

	event->write_index = reg.write_index;
	event->status_index = reg.status_index;
	event->group = group;

	event->event_external.enable = &event->group->mmap[event->status_index];

	[..]

	return &event->event_external;

All the other functions wouldn't even have to do a container_of() call, as
the event_external will be the first field in the struct it needs.

	struct tracefs_user_event_internal *event = (struct tracefs_user_event_internal *)event_external;

Or make the above a helper function:

#define INTERNAL_EVENT(e) ((struct tracefs_user_event_internal *)e)

	struct tracefs_user_event_internal *event = INTERNAL_EVENT(event_external);


Then we save on the function call, and allow the code to do the test inline.

-- Steve