[PATCH v3 0/5] Refactor the APIs for tracing options

[PATCH v3 0/5] Refactor the APIs for tracing options

[Yordan Karadzhov (VMware)]

Yordan Karadzhov (VMware) (5):
  libtracefs: Fix issues with tracefs_option_id()
  libtracefs: Modify the arguments of tracefs_option_is_set()
  libtracefs: Encapsulate "struct tracefs_options_mask"
  libtracefs: Option's bit masks to be owned by the instance
  libtracefs: Rename tracefs_option_is_set()

 Documentation/libtracefs-option-get.txt |  4 +-
 include/tracefs-local.h                 | 12 ++++++
 include/tracefs.h                       | 11 ++---
 src/tracefs-instance.c                  | 15 +++++++
 src/tracefs-tools.c                     | 57 +++++++++----------------
 utest/tracefs-utest.c                   | 12 ++----
 6 files changed, 56 insertions(+), 55 deletions(-)

-- 
2.27.0

[PATCH v3 1/5] libtracefs: Fix issues with tracefs_option_id()

[Yordan Karadzhov (VMware)]

Few mistakes have been made when introducing this API function.
First of all, its declaration is missing from the header file.
In addition to this the argument type is missing "const".

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs.h   | 1 +
 src/tracefs-tools.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/tracefs.h b/include/tracefs.h
index 70b7ebe..d3db8b5 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -145,6 +145,7 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
 int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id);
 int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id);
 const char *tracefs_option_name(enum tracefs_option_id id);
+enum tracefs_option_id tracefs_option_id(const char *name);
 
 /*
  * RESET	- Reset on opening filter file (O_TRUNC)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index d48062c..f409def 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -192,7 +192,7 @@ const char *tracefs_option_name(enum tracefs_option_id id)
  * Returns trace option ID or TRACEFS_OPTION_INVALID in case of an error or
  * unknown option name.
  */
-enum tracefs_option_id tracefs_option_id(char *name)
+enum tracefs_option_id tracefs_option_id(const char *name)
 {
 	int i;
 
-- 
2.27.0

[PATCH v3 2/5] libtracefs: Modify the arguments of tracefs_option_is_set()

[Yordan Karadzhov (VMware)]

This function is supposed to take as argument a mask returned by
"tracefs_options_get_supported()" or "tracefs_options_get_enabled()",
hence this argument must be a pointer.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs.h     |  4 ++--
 src/tracefs-tools.c   |  5 +++--
 utest/tracefs-utest.c | 12 ++++++------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/include/tracefs.h b/include/tracefs.h
index d3db8b5..95016c8 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -136,8 +136,8 @@ struct tracefs_options_mask {
 };
 void tracefs_option_set(struct tracefs_options_mask *options, enum tracefs_option_id id);
 void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_option_id id);
-bool tracefs_option_is_set(struct tracefs_options_mask options, enum tracefs_option_id id);
-
+bool tracefs_option_is_set(struct tracefs_options_mask *options,
+			   enum tracefs_option_id id);
 struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance);
 bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id);
 struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance);
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index f409def..25c0cea 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -370,10 +370,11 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
  * Returns true if an option with given id is set in the bitmask,
  * false if it is not set.
  */
-bool tracefs_option_is_set(struct tracefs_options_mask options, enum tracefs_option_id id)
+bool tracefs_option_is_set(struct tracefs_options_mask *options,
+			   enum tracefs_option_id id)
 {
 	if (id > TRACEFS_OPTION_INVALID)
-		return options.mask & (1ULL << (id - 1));
+		return options->mask & (1ULL << (id - 1));
 	return false;
 }
 
diff --git a/utest/tracefs-utest.c b/utest/tracefs-utest.c
index ed2693b..9cb2a09 100644
--- a/utest/tracefs-utest.c
+++ b/utest/tracefs-utest.c
@@ -547,7 +547,7 @@ static bool check_options_mask_empty(struct tracefs_options_mask *mask)
 	int i;
 
 	for (i = 1; i < TRACEFS_OPTION_MAX; i++) {
-		if (tracefs_option_is_set(*mask, i))
+		if (tracefs_option_is_set(mask, i))
 			return false;
 	}
 	return true;
@@ -580,9 +580,9 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 		CU_TEST(strcmp(name, "unknown"));
 		snprintf(file, PATH_MAX, "options/%s", name);
 
-		if (tracefs_option_is_set(*all, i)) {
+		if (tracefs_option_is_set(all, i)) {
 			tracefs_option_clear(all, i);
-			CU_TEST(!tracefs_option_is_set(*all, i));
+			CU_TEST(!tracefs_option_is_set(all, i));
 			CU_TEST(check_option(instance, i, true, -1));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 		} else {
@@ -590,9 +590,9 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 			CU_TEST(!tracefs_option_is_supported(instance, i));
 		}
 
-		if (tracefs_option_is_set(*enabled, i)) {
+		if (tracefs_option_is_set(enabled, i)) {
 			tracefs_option_clear(enabled, i);
-			CU_TEST(!tracefs_option_is_set(*enabled, i));
+			CU_TEST(!tracefs_option_is_set(enabled, i));
 			CU_TEST(check_option(instance, i, true, 1));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 			CU_TEST(tracefs_option_is_enabled(instance, i));
@@ -600,7 +600,7 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 			CU_TEST(check_option(instance, i, true, 0));
 			CU_TEST(tracefs_option_enable(instance, i) == 0);
 			CU_TEST(check_option(instance, i, true, 1));
-		} else if (tracefs_option_is_set(*all_copy, i)) {
+		} else if (tracefs_option_is_set(all_copy, i)) {
 			CU_TEST(check_option(instance, i, true, 0));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 			CU_TEST(!tracefs_option_is_enabled(instance, i));
-- 
2.27.0

[PATCH v3 3/5] libtracefs: Encapsulate "struct tracefs_options_mask"

[Yordan Karadzhov (VMware)]

The definition of the mask gets hidden from the user. This way we will
be able to modify this definition in the future, without breaking the
API. Such a modification will be compulsory, if too many new tracing
options are added in the future. Note that encapsulating the mask
definition, requires two API methods to be eliminated, however those
methods have no particular use-cases for the moment.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs-local.h |  4 ++++
 include/tracefs.h       |  6 +-----
 src/tracefs-tools.c     | 26 ++------------------------
 utest/tracefs-utest.c   |  4 ----
 4 files changed, 7 insertions(+), 33 deletions(-)

diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 6865611..076b013 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -14,6 +14,10 @@
 #define BUILD_BUG_ON(cond)			\
 	do { if (!(1/!(cond))) { } } while (0)
 
+struct tracefs_options_mask {
+	unsigned long long	mask;
+};
+
 struct tracefs_instance {
 	char			*trace_dir;
 	char			*name;
diff --git a/include/tracefs.h b/include/tracefs.h
index 95016c8..e1fbef6 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -131,11 +131,7 @@ enum tracefs_option_id {
 };
 #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1)
 
-struct tracefs_options_mask {
-	unsigned long long	mask;
-};
-void tracefs_option_set(struct tracefs_options_mask *options, enum tracefs_option_id id);
-void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_option_id id);
+struct tracefs_options_mask;
 bool tracefs_option_is_set(struct tracefs_options_mask *options,
 			   enum tracefs_option_id id);
 struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance);
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 25c0cea..fc9644a 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -239,8 +239,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
 				continue;
 		}
 		id = tracefs_option_id(dent->d_name);
-		if (id != TRACEFS_OPTION_INVALID)
-			tracefs_option_set(bitmask, id);
+		if (id > TRACEFS_OPTION_INVALID)
+			bitmask->mask |= (1ULL << (id - 1));
 	}
 	closedir(dir);
 	tracefs_put_tracing_file(dname);
@@ -378,28 +378,6 @@ bool tracefs_option_is_set(struct tracefs_options_mask *options,
 	return false;
 }
 
-/**
- * tracefs_option_set - Set option in options bitmask
- * @options: Pointer to a bitmask with options
- * @id: trace option id
- */
-void tracefs_option_set(struct tracefs_options_mask *options, enum tracefs_option_id id)
-{
-	if (options && id > TRACEFS_OPTION_INVALID)
-		options->mask |= (1ULL << (id - 1));
-}
-
-/**
- * tracefs_option_clear - Clear option from options bitmask
- * @options: Pointer to a bitmask with options
- * @id: trace option id
- */
-void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_option_id id)
-{
-	if (options && id > TRACEFS_OPTION_INVALID)
-		options->mask &= ~(1ULL << (id - 1));
-}
-
 struct func_list {
 	struct func_list	*next;
 	unsigned int		start;
diff --git a/utest/tracefs-utest.c b/utest/tracefs-utest.c
index 9cb2a09..af28779 100644
--- a/utest/tracefs-utest.c
+++ b/utest/tracefs-utest.c
@@ -581,8 +581,6 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 		snprintf(file, PATH_MAX, "options/%s", name);
 
 		if (tracefs_option_is_set(all, i)) {
-			tracefs_option_clear(all, i);
-			CU_TEST(!tracefs_option_is_set(all, i));
 			CU_TEST(check_option(instance, i, true, -1));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 		} else {
@@ -591,8 +589,6 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 		}
 
 		if (tracefs_option_is_set(enabled, i)) {
-			tracefs_option_clear(enabled, i);
-			CU_TEST(!tracefs_option_is_set(enabled, i));
 			CU_TEST(check_option(instance, i, true, 1));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 			CU_TEST(tracefs_option_is_enabled(instance, i));
-- 
2.27.0

[PATCH v3 4/5] libtracefs: Option's bit masks to be owned by the instance

[Yordan Karadzhov (VMware)]

If the instance owns two mask objects, we no longer need to
dynamically allocate memory in tracefs_options_get_supported() and
tracefs_options_get_enabled(). This will simplify the code on the
caller side, since the user is no longer responsible for freeing
those masks.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 Documentation/libtracefs-option-get.txt |  4 +---
 include/tracefs-local.h                 |  8 ++++++++
 include/tracefs.h                       |  2 +-
 src/tracefs-instance.c                  | 15 +++++++++++++++
 src/tracefs-tools.c                     | 24 ++++++++++++++----------
 5 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt
index 9b3cb56..3290f24 100644
--- a/Documentation/libtracefs-option-get.txt
+++ b/Documentation/libtracefs-option-get.txt
@@ -52,14 +52,13 @@ EXAMPLE
 --
 #include <tracefs.h>
 ...
-struct tracefs_options_mask *options;
+const struct tracefs_options_mask *options;
 ...
 options = tracefs_options_get_supported(NULL);
 if (!options) {
 	/* Failed to get supported options */
 } else {
 	...
-	free(options);
 }
 ...
 options = tracefs_options_get_enabled(NULL);
@@ -67,7 +66,6 @@ if (!options) {
 	/* Failed to get options, enabled in the top instance */
 } else {
 	...
-	free(options);
 }
 ...
 
diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 076b013..eb8c81b 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -25,6 +25,8 @@ struct tracefs_instance {
 	int			flags;
 	int			ftrace_filter_fd;
 	int			ftrace_notrace_fd;
+	struct tracefs_options_mask	supported_opts;
+	struct tracefs_options_mask	enabled_opts;
 };
 
 extern pthread_mutex_t toplevel_lock;
@@ -48,4 +50,10 @@ char *trace_find_tracing_dir(void);
 #define DEFFILEMODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH) /* 0666*/
 #endif
 
+struct tracefs_options_mask *
+supported_opts_mask(struct tracefs_instance *instance);
+
+struct tracefs_options_mask *
+enabled_opts_mask(struct tracefs_instance *instance);
+
 #endif /* _TRACE_FS_LOCAL_H */
diff --git a/include/tracefs.h b/include/tracefs.h
index e1fbef6..ea9dbd0 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -132,7 +132,7 @@ enum tracefs_option_id {
 #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1)
 
 struct tracefs_options_mask;
-bool tracefs_option_is_set(struct tracefs_options_mask *options,
+bool tracefs_option_is_set(const struct tracefs_options_mask *options,
 			   enum tracefs_option_id id);
 struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance);
 bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id);
diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
index 599c3a7..74122aa 100644
--- a/src/tracefs-instance.c
+++ b/src/tracefs-instance.c
@@ -21,6 +21,21 @@
 
 #define FLAG_INSTANCE_NEWLY_CREATED	(1 << 0)
 
+struct tracefs_options_mask	toplevel_supported_opts;
+struct tracefs_options_mask	toplevel_enabled_opts;
+
+__hidden inline struct tracefs_options_mask *
+supported_opts_mask(struct tracefs_instance *instance)
+{
+	return instance? &instance->supported_opts : &toplevel_supported_opts;
+}
+
+__hidden inline struct tracefs_options_mask *
+enabled_opts_mask(struct tracefs_instance *instance)
+{
+	return instance? &instance->enabled_opts : &toplevel_enabled_opts;
+}
+
 /**
  * instance_alloc - allocate a new ftrace instance
  * @trace_dir - Full path to the tracing directory, where the instance is
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index fc9644a..b6967db 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -211,6 +211,7 @@ enum tracefs_option_id tracefs_option_id(const char *name)
 static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *instance,
 						      bool enabled)
 {
+	pthread_mutex_t *lock = instance ? &instance->lock : &toplevel_lock;
 	struct tracefs_options_mask *bitmask;
 	enum tracefs_option_id id;
 	char file[PATH_MAX];
@@ -219,9 +220,6 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
 	DIR *dir = NULL;
 	long long val;
 
-	bitmask = calloc(1, sizeof(struct tracefs_options_mask));
-	if (!bitmask)
-		return NULL;
 	dname = tracefs_instance_get_file(instance, "options");
 	if (!dname)
 		goto error;
@@ -229,6 +227,11 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
 	if (!dir)
 		goto error;
 
+	bitmask = enabled? enabled_opts_mask(instance) :
+			   supported_opts_mask(instance);
+
+	pthread_mutex_lock(lock);
+	bitmask->mask = 0;
 	while ((dent = readdir(dir))) {
 		if (*dent->d_name == '.')
 			continue;
@@ -242,6 +245,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
 		if (id > TRACEFS_OPTION_INVALID)
 			bitmask->mask |= (1ULL << (id - 1));
 	}
+	pthread_mutex_unlock(lock);
+
 	closedir(dir);
 	tracefs_put_tracing_file(dname);
 
@@ -251,7 +256,6 @@ error:
 	if (dir)
 		closedir(dir);
 	tracefs_put_tracing_file(dname);
-	free(bitmask);
 	return NULL;
 }
 
@@ -259,8 +263,8 @@ error:
  * tracefs_options_get_supported - Get all supported trace options in given instance
  * @instance: ftrace instance, can be NULL for the top instance
  *
- * Returns allocated bitmask structure with all trace options, supported in given
- * instance, or NULL in case of an error. The returned structure must be freed with free()
+ * Returns bitmask structure with all trace options, supported in given instance,
+ * or NULL in case of an error.
  */
 struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance)
 {
@@ -271,8 +275,8 @@ struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instan
  * tracefs_options_get_enabled - Get all currently enabled trace options in given instance
  * @instance: ftrace instance, can be NULL for the top instance
  *
- * Returns allocated bitmask structure with all trace options, enabled in given
- * instance, or NULL in case of an error. The returned structure must be freed with free()
+ * Returns bitmask structure with all trace options, enabled in given instance,
+ * or NULL in case of an error.
  */
 struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance)
 {
@@ -282,7 +286,7 @@ struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance
 static int trace_config_option(struct tracefs_instance *instance,
 			       enum tracefs_option_id id, bool set)
 {
-	char *set_str = set ? "1" : "0";
+	const char *set_str = set ? "1" : "0";
 	char file[PATH_MAX];
 	const char *name;
 
@@ -370,7 +374,7 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
  * Returns true if an option with given id is set in the bitmask,
  * false if it is not set.
  */
-bool tracefs_option_is_set(struct tracefs_options_mask *options,
+bool tracefs_option_is_set(const struct tracefs_options_mask *options,
 			   enum tracefs_option_id id)
 {
 	if (id > TRACEFS_OPTION_INVALID)
-- 
2.27.0

Re: [PATCH v3 4/5] libtracefs: Option's bit masks to be owned by the instance

[Tzvetomir Stoyanov]

On Thu, Apr 8, 2021 at 5:00 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
> If the instance owns two mask objects, we no longer need to
> dynamically allocate memory in tracefs_options_get_supported() and
> tracefs_options_get_enabled(). This will simplify the code on the
> caller side, since the user is no longer responsible for freeing
> those masks.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  Documentation/libtracefs-option-get.txt |  4 +---
>  include/tracefs-local.h                 |  8 ++++++++
>  include/tracefs.h                       |  2 +-
>  src/tracefs-instance.c                  | 15 +++++++++++++++
>  src/tracefs-tools.c                     | 24 ++++++++++++++----------
>  5 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt
> index 9b3cb56..3290f24 100644
> --- a/Documentation/libtracefs-option-get.txt
> +++ b/Documentation/libtracefs-option-get.txt
> @@ -52,14 +52,13 @@ EXAMPLE
>  --
>  #include <tracefs.h>
>  ...
> -struct tracefs_options_mask *options;
> +const struct tracefs_options_mask *options;
>  ...
>  options = tracefs_options_get_supported(NULL);
>  if (!options) {
>         /* Failed to get supported options */
>  } else {
>         ...
> -       free(options);
>  }
>  ...
>  options = tracefs_options_get_enabled(NULL);
> @@ -67,7 +66,6 @@ if (!options) {
>         /* Failed to get options, enabled in the top instance */
>  } else {
>         ...
> -       free(options);
>  }
>  ...
The description in the "RETURN VALUE" section should be changed also,
as now it suggests to free the bitmask returned by those APIs.

>
> diff --git a/include/tracefs-local.h b/include/tracefs-local.h
> index 076b013..eb8c81b 100644
> --- a/include/tracefs-local.h
> +++ b/include/tracefs-local.h
> @@ -25,6 +25,8 @@ struct tracefs_instance {
>         int                     flags;
>         int                     ftrace_filter_fd;
>         int                     ftrace_notrace_fd;
> +       struct tracefs_options_mask     supported_opts;
> +       struct tracefs_options_mask     enabled_opts;
>  };
>
>  extern pthread_mutex_t toplevel_lock;
> @@ -48,4 +50,10 @@ char *trace_find_tracing_dir(void);
>  #define DEFFILEMODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH) /* 0666*/
>  #endif
>
> +struct tracefs_options_mask *
> +supported_opts_mask(struct tracefs_instance *instance);
> +
> +struct tracefs_options_mask *
> +enabled_opts_mask(struct tracefs_instance *instance);
> +
>  #endif /* _TRACE_FS_LOCAL_H */
> diff --git a/include/tracefs.h b/include/tracefs.h
> index e1fbef6..ea9dbd0 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -132,7 +132,7 @@ enum tracefs_option_id {
>  #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1)
>
>  struct tracefs_options_mask;
> -bool tracefs_option_is_set(struct tracefs_options_mask *options,
> +bool tracefs_option_is_set(const struct tracefs_options_mask *options,
>                            enum tracefs_option_id id);
>  struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance);
>  bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id);
> diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
> index 599c3a7..74122aa 100644
> --- a/src/tracefs-instance.c
> +++ b/src/tracefs-instance.c
> @@ -21,6 +21,21 @@
>
>  #define FLAG_INSTANCE_NEWLY_CREATED    (1 << 0)
>
> +struct tracefs_options_mask    toplevel_supported_opts;
> +struct tracefs_options_mask    toplevel_enabled_opts;
> +
> +__hidden inline struct tracefs_options_mask *
> +supported_opts_mask(struct tracefs_instance *instance)
> +{
> +       return instance? &instance->supported_opts : &toplevel_supported_opts;
> +}
> +
> +__hidden inline struct tracefs_options_mask *
> +enabled_opts_mask(struct tracefs_instance *instance)
> +{
> +       return instance? &instance->enabled_opts : &toplevel_enabled_opts;
> +}
> +
>  /**
>   * instance_alloc - allocate a new ftrace instance
>   * @trace_dir - Full path to the tracing directory, where the instance is
> diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
> index fc9644a..b6967db 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -211,6 +211,7 @@ enum tracefs_option_id tracefs_option_id(const char *name)
>  static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *instance,
>                                                       bool enabled)
>  {
> +       pthread_mutex_t *lock = instance ? &instance->lock : &toplevel_lock;
>         struct tracefs_options_mask *bitmask;
>         enum tracefs_option_id id;
>         char file[PATH_MAX];
> @@ -219,9 +220,6 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
>         DIR *dir = NULL;
>         long long val;
>
> -       bitmask = calloc(1, sizeof(struct tracefs_options_mask));
> -       if (!bitmask)
> -               return NULL;
>         dname = tracefs_instance_get_file(instance, "options");
>         if (!dname)
>                 goto error;
> @@ -229,6 +227,11 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
>         if (!dir)
>                 goto error;
>
> +       bitmask = enabled? enabled_opts_mask(instance) :
> +                          supported_opts_mask(instance);
> +
> +       pthread_mutex_lock(lock);
> +       bitmask->mask = 0;
>         while ((dent = readdir(dir))) {
>                 if (*dent->d_name == '.')
>                         continue;
> @@ -242,6 +245,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i
>                 if (id > TRACEFS_OPTION_INVALID)
>                         bitmask->mask |= (1ULL << (id - 1));
>         }
> +       pthread_mutex_unlock(lock);
> +
>         closedir(dir);
>         tracefs_put_tracing_file(dname);
>
> @@ -251,7 +256,6 @@ error:
>         if (dir)
>                 closedir(dir);
>         tracefs_put_tracing_file(dname);
> -       free(bitmask);
>         return NULL;
>  }
>
> @@ -259,8 +263,8 @@ error:
>   * tracefs_options_get_supported - Get all supported trace options in given instance
>   * @instance: ftrace instance, can be NULL for the top instance
>   *
> - * Returns allocated bitmask structure with all trace options, supported in given
> - * instance, or NULL in case of an error. The returned structure must be freed with free()
> + * Returns bitmask structure with all trace options, supported in given instance,
> + * or NULL in case of an error.
>   */
>  struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance)
>  {
> @@ -271,8 +275,8 @@ struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instan
>   * tracefs_options_get_enabled - Get all currently enabled trace options in given instance
>   * @instance: ftrace instance, can be NULL for the top instance
>   *
> - * Returns allocated bitmask structure with all trace options, enabled in given
> - * instance, or NULL in case of an error. The returned structure must be freed with free()
> + * Returns bitmask structure with all trace options, enabled in given instance,
> + * or NULL in case of an error.
>   */
>  struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance)
>  {
> @@ -282,7 +286,7 @@ struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance
>  static int trace_config_option(struct tracefs_instance *instance,
>                                enum tracefs_option_id id, bool set)
>  {
> -       char *set_str = set ? "1" : "0";
> +       const char *set_str = set ? "1" : "0";
>         char file[PATH_MAX];
>         const char *name;
>
> @@ -370,7 +374,7 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
>   * Returns true if an option with given id is set in the bitmask,
>   * false if it is not set.
>   */
> -bool tracefs_option_is_set(struct tracefs_options_mask *options,
> +bool tracefs_option_is_set(const struct tracefs_options_mask *options,
>                            enum tracefs_option_id id)
>  {
>         if (id > TRACEFS_OPTION_INVALID)
> --
> 2.27.0
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

[PATCH v3 5/5] libtracefs: Rename tracefs_option_is_set()

[Yordan Karadzhov (VMware)]

The old name of the function is potentially confusing. Indeed
the function do not check anything about the option itself (is it
supported or is it enabled). The function only check if inside
the mask the bit that corresponds to a given option is set.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs.h     | 4 ++--
 src/tracefs-tools.c   | 4 ++--
 utest/tracefs-utest.c | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/tracefs.h b/include/tracefs.h
index ea9dbd0..daae419 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -132,8 +132,8 @@ enum tracefs_option_id {
 #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1)
 
 struct tracefs_options_mask;
-bool tracefs_option_is_set(const struct tracefs_options_mask *options,
-			   enum tracefs_option_id id);
+bool tracefs_option_mask_is_set(const struct tracefs_options_mask *options,
+				enum tracefs_option_id id);
 struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance);
 bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id);
 struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance);
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index b6967db..126fc57 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -367,14 +367,14 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
 }
 
 /**
- * tracefs_option_is_set - Check if given option is set in the bitmask
+ * tracefs_option_mask_is_set - Check if given option is set in the bitmask
  * @options: Options bitmask
  * @id: trace option id
  *
  * Returns true if an option with given id is set in the bitmask,
  * false if it is not set.
  */
-bool tracefs_option_is_set(const struct tracefs_options_mask *options,
+bool tracefs_option_mask_is_set(const struct tracefs_options_mask *options,
 			   enum tracefs_option_id id)
 {
 	if (id > TRACEFS_OPTION_INVALID)
diff --git a/utest/tracefs-utest.c b/utest/tracefs-utest.c
index af28779..185ac2a 100644
--- a/utest/tracefs-utest.c
+++ b/utest/tracefs-utest.c
@@ -547,7 +547,7 @@ static bool check_options_mask_empty(struct tracefs_options_mask *mask)
 	int i;
 
 	for (i = 1; i < TRACEFS_OPTION_MAX; i++) {
-		if (tracefs_option_is_set(mask, i))
+		if (tracefs_option_mask_is_set(mask, i))
 			return false;
 	}
 	return true;
@@ -580,7 +580,7 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 		CU_TEST(strcmp(name, "unknown"));
 		snprintf(file, PATH_MAX, "options/%s", name);
 
-		if (tracefs_option_is_set(all, i)) {
+		if (tracefs_option_mask_is_set(all, i)) {
 			CU_TEST(check_option(instance, i, true, -1));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 		} else {
@@ -588,7 +588,7 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 			CU_TEST(!tracefs_option_is_supported(instance, i));
 		}
 
-		if (tracefs_option_is_set(enabled, i)) {
+		if (tracefs_option_mask_is_set(enabled, i)) {
 			CU_TEST(check_option(instance, i, true, 1));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 			CU_TEST(tracefs_option_is_enabled(instance, i));
@@ -596,7 +596,7 @@ static void test_instance_tracing_options(struct tracefs_instance *instance)
 			CU_TEST(check_option(instance, i, true, 0));
 			CU_TEST(tracefs_option_enable(instance, i) == 0);
 			CU_TEST(check_option(instance, i, true, 1));
-		} else if (tracefs_option_is_set(all_copy, i)) {
+		} else if (tracefs_option_mask_is_set(all_copy, i)) {
 			CU_TEST(check_option(instance, i, true, 0));
 			CU_TEST(tracefs_option_is_supported(instance, i));
 			CU_TEST(!tracefs_option_is_enabled(instance, i));
-- 
2.27.0

Re: [PATCH v3 0/5] Refactor the APIs for tracing options

[Tzvetomir Stoyanov]

On Thu, Apr 8, 2021 at 5:00 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
> Yordan Karadzhov (VMware) (5):
>   libtracefs: Fix issues with tracefs_option_id()
>   libtracefs: Modify the arguments of tracefs_option_is_set()
>   libtracefs: Encapsulate "struct tracefs_options_mask"
>   libtracefs: Option's bit masks to be owned by the instance
>   libtracefs: Rename tracefs_option_is_set()
>
>  Documentation/libtracefs-option-get.txt |  4 +-
>  include/tracefs-local.h                 | 12 ++++++
>  include/tracefs.h                       | 11 ++---
>  src/tracefs-instance.c                  | 15 +++++++
>  src/tracefs-tools.c                     | 57 +++++++++----------------
>  utest/tracefs-utest.c                   | 12 ++----
>  6 files changed, 56 insertions(+), 55 deletions(-)
>
> --
> 2.27.0
>
The man pages should be updated to reflect the changes in the APIs.

-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center