On Tue, Feb 22, 2022 at 12:31:43PM -0500, Steven Rostedt wrote:
> 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.
Yes, I was thinking along the same lines. I saw how things were split
between private / public in the headers and wasn't sure how to
accomplish this.
What you have above seems like a great way to accomplish this without exposing
too much out.
Thanks,
-Beau
|