diff mbox series

[v3,01/11] libtracefs: Add new public macros for bits manipulations

Message ID 20211103154417.246999-2-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series libtracefs dynamic events support | expand

Commit Message

Tzvetomir Stoyanov (VMware) Nov. 3, 2021, 3:44 p.m. UTC
Some of the tracefs library APIs may have bit mask parameters. To
simplify the usage of such APIs, generic bit manipulations macros are
added to the library API:
 TRACEFS_BIT_SET
 TRACEFS_BIT_TEST
 TRACEFS_BIT_CLEAR

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/tracefs.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Steven Rostedt Nov. 3, 2021, 4:24 p.m. UTC | #1
On Wed,  3 Nov 2021 17:44:07 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> diff --git a/include/tracefs.h b/include/tracefs.h
> index a2cda30..fa7f316 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -10,6 +10,10 @@
>  #include <sched.h>
>  #include <event-parse.h>
>  
> +#define TRACEFS_BIT_SET(M, B)	do { (M) |= (1ULL << (B)); } while (0)
> +#define TRACEFS_BIT_TEST(M, B)	((M) & (1ULL<<(B)))
> +#define TRACEFS_BIT_CLEAR(M, B)	do { (M) &= ~(1ULL << (B)); } while (0)

Does this really need to be public?

I was thinking that this would just be used internally, and thus in the
tracefs-local.h.

-- Steve

> +
>  char *tracefs_get_tracing_file(const char *name);
Tzvetomir Stoyanov (VMware) Nov. 3, 2021, 4:34 p.m. UTC | #2
On Wed, Nov 3, 2021 at 6:24 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed,  3 Nov 2021 17:44:07 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > diff --git a/include/tracefs.h b/include/tracefs.h
> > index a2cda30..fa7f316 100644
> > --- a/include/tracefs.h
> > +++ b/include/tracefs.h
> > @@ -10,6 +10,10 @@
> >  #include <sched.h>
> >  #include <event-parse.h>
> >
> > +#define TRACEFS_BIT_SET(M, B)        do { (M) |= (1ULL << (B)); } while (0)
> > +#define TRACEFS_BIT_TEST(M, B)       ((M) & (1ULL<<(B)))
> > +#define TRACEFS_BIT_CLEAR(M, B)      do { (M) &= ~(1ULL << (B)); } while (0)
>
> Does this really need to be public?
>
> I was thinking that this would just be used internally, and thus in the
> tracefs-local.h.
>

The  only reason to export it is because of these APIs, which get
bitmask of enum tracefs_dynevent_type types:
     int tracefs_dynevent_destroy_all(unsigned long type_mask, bool force);
     struct tracefs_dynevent **tracefs_dynevent_get_all(unsigned long
type_mask, const char *system);
Currently that type is simple enum, and those macros are used to
construct the bitmask:
enum tracefs_dynevent_type {
    TRACEFS_DYNEVENT_KPROBE = 0,
    TRACEFS_DYNEVENT_KRETPROBE,
    TRACEFS_DYNEVENT_UPROBE,
    TRACEFS_DYNEVENT_URETPROBE,
    TRACEFS_DYNEVENT_EPROBE,
    TRACEFS_DYNEVENT_SYNTH,
    TRACEFS_DYNEVENT_MAX,
};

The other possible approach is to define the eum in that way, then no
macros will be needed:
enum tracefs_dynevent_type {
    TRACEFS_DYNEVENT_KPROBE = 1,
    TRACEFS_DYNEVENT_KRETPROBE = 1 << 1,
    TRACEFS_DYNEVENT_UPROBE = 1 << 2,
    TRACEFS_DYNEVENT_URETPROBE = 1 << 3,
    TRACEFS_DYNEVENT_EPROBE = 1 << 4,
    TRACEFS_DYNEVENT_SYNTH = 1 << 5,
    TRACEFS_DYNEVENT_MAX = 0,
};

> -- Steve
>
> > +
> >  char *tracefs_get_tracing_file(const char *name);
Steven Rostedt Nov. 3, 2021, 4:47 p.m. UTC | #3
On Wed, 3 Nov 2021 18:34:49 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> On Wed, Nov 3, 2021 at 6:24 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Wed,  3 Nov 2021 17:44:07 +0200
> > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> >  
> > > diff --git a/include/tracefs.h b/include/tracefs.h
> > > index a2cda30..fa7f316 100644
> > > --- a/include/tracefs.h
> > > +++ b/include/tracefs.h
> > > @@ -10,6 +10,10 @@
> > >  #include <sched.h>
> > >  #include <event-parse.h>
> > >
> > > +#define TRACEFS_BIT_SET(M, B)        do { (M) |= (1ULL << (B)); } while (0)
> > > +#define TRACEFS_BIT_TEST(M, B)       ((M) & (1ULL<<(B)))
> > > +#define TRACEFS_BIT_CLEAR(M, B)      do { (M) &= ~(1ULL << (B)); } while (0)  
> >
> > Does this really need to be public?
> >
> > I was thinking that this would just be used internally, and thus in the
> > tracefs-local.h.
> >  
> 
> The  only reason to export it is because of these APIs, which get
> bitmask of enum tracefs_dynevent_type types:
>      int tracefs_dynevent_destroy_all(unsigned long type_mask, bool force);
>      struct tracefs_dynevent **tracefs_dynevent_get_all(unsigned long
> type_mask, const char *system);
> Currently that type is simple enum, and those macros are used to
> construct the bitmask:
> enum tracefs_dynevent_type {
>     TRACEFS_DYNEVENT_KPROBE = 0,
>     TRACEFS_DYNEVENT_KRETPROBE,
>     TRACEFS_DYNEVENT_UPROBE,
>     TRACEFS_DYNEVENT_URETPROBE,
>     TRACEFS_DYNEVENT_EPROBE,
>     TRACEFS_DYNEVENT_SYNTH,
>     TRACEFS_DYNEVENT_MAX,
> };
> 
> The other possible approach is to define the eum in that way, then no
> macros will be needed:
> enum tracefs_dynevent_type {
>     TRACEFS_DYNEVENT_KPROBE = 1,

For consistency we usually use "1 << 0" for this.

>     TRACEFS_DYNEVENT_KRETPROBE = 1 << 1,
>     TRACEFS_DYNEVENT_UPROBE = 1 << 2,
>     TRACEFS_DYNEVENT_URETPROBE = 1 << 3,
>     TRACEFS_DYNEVENT_EPROBE = 1 << 4,
>     TRACEFS_DYNEVENT_SYNTH = 1 << 5,

The above is very common, but make sure to tab it:

     TRACEFS_DYNEVENT_KPROBE		= 1 << 0,
     TRACEFS_DYNEVENT_KRETPROBE		= 1 << 1,
     TRACEFS_DYNEVENT_UPROBE		= 1 << 2,
     TRACEFS_DYNEVENT_URETPROBE		= 1 << 3,
     TRACEFS_DYNEVENT_EPROBE		= 1 << 4,
     TRACEFS_DYNEVENT_SYNTH		= 1 << 5,


>     TRACEFS_DYNEVENT_MAX = 0,

Although 0 does not make sense. What you do is this:

	TRACEFS_DYNEVENT_MAX		= 1 << 6;

And then you can also have:

#define TRACEFS_DYNEVENT_MASK (TRACEFS_DYNEVENT_MAX - 1)

-- Steve
diff mbox series

Patch

diff --git a/include/tracefs.h b/include/tracefs.h
index a2cda30..fa7f316 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -10,6 +10,10 @@ 
 #include <sched.h>
 #include <event-parse.h>
 
+#define TRACEFS_BIT_SET(M, B)	do { (M) |= (1ULL << (B)); } while (0)
+#define TRACEFS_BIT_TEST(M, B)	((M) & (1ULL<<(B)))
+#define TRACEFS_BIT_CLEAR(M, B)	do { (M) &= ~(1ULL << (B)); } while (0)
+
 char *tracefs_get_tracing_file(const char *name);
 void tracefs_put_tracing_file(char *name);