linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Refactor the APIs for tracing options
@ 2021-04-09 18:04 Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 1/5] libtracefs: Fix issues with tracefs_option_id() Yordan Karadzhov (VMware)
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-04-09 18:04 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, tz.stoyanov, 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-bits.txt |  10 +--
 Documentation/libtracefs-option-get.txt  |   7 +-
 include/tracefs-local.h                  |  12 +++
 include/tracefs.h                        |  11 +--
 src/tracefs-instance.c                   |  15 ++++
 src/tracefs-tools.c                      | 103 +++++++++--------------
 utest/tracefs-utest.c                    |  12 +--
 7 files changed, 81 insertions(+), 89 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4 1/5] libtracefs: Fix issues with tracefs_option_id()
  2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
@ 2021-04-09 18:04 ` Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 2/5] libtracefs: Modify the arguments of tracefs_option_is_set() Yordan Karadzhov (VMware)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-04-09 18:04 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, tz.stoyanov, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 2/5] libtracefs: Modify the arguments of tracefs_option_is_set()
  2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 1/5] libtracefs: Fix issues with tracefs_option_id() Yordan Karadzhov (VMware)
@ 2021-04-09 18:04 ` Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 3/5] libtracefs: Encapsulate "struct tracefs_options_mask" Yordan Karadzhov (VMware)
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-04-09 18:04 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, tz.stoyanov, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 3/5] libtracefs: Encapsulate "struct tracefs_options_mask"
  2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 1/5] libtracefs: Fix issues with tracefs_option_id() Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 2/5] libtracefs: Modify the arguments of tracefs_option_is_set() Yordan Karadzhov (VMware)
@ 2021-04-09 18:04 ` Yordan Karadzhov (VMware)
  2021-04-09 18:04 ` [PATCH v4 4/5] libtracefs: Option's bit masks to be owned by the instance Yordan Karadzhov (VMware)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-04-09 18:04 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, tz.stoyanov, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 4/5] libtracefs: Option's bit masks to be owned by the instance
  2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
                   ` (2 preceding siblings ...)
  2021-04-09 18:04 ` [PATCH v4 3/5] libtracefs: Encapsulate "struct tracefs_options_mask" Yordan Karadzhov (VMware)
@ 2021-04-09 18:04 ` Yordan Karadzhov (VMware)
  2021-04-09 18:40   ` Steven Rostedt
  2021-04-09 18:04 ` [PATCH v4 5/5] libtracefs: Rename tracefs_option_is_set() Yordan Karadzhov (VMware)
  2021-04-09 18:26 ` [PATCH v4 0/5] Refactor the APIs for tracing options Steven Rostedt
  5 siblings, 1 reply; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-04-09 18:04 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, tz.stoyanov, 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 |  7 +--
 include/tracefs-local.h                 |  8 +++
 include/tracefs.h                       |  2 +-
 src/tracefs-instance.c                  | 15 +++++
 src/tracefs-tools.c                     | 74 ++++++++++++-------------
 5 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt
index 9b3cb56..4f5291e 100644
--- a/Documentation/libtracefs-option-get.txt
+++ b/Documentation/libtracefs-option-get.txt
@@ -40,8 +40,7 @@ given _instance_. If _instance_ is NULL, the top trace instance is used.
 RETURN VALUE
 ------------
 The _tracefs_options_get_supported()_ and _tracefs_options_get_enabled()_ functions return pointer
-to allocated bitmask with trace options, or NULL in case of an error. The returned bitmask must be
-freed with free();
+to allocated bitmask with trace options, or NULL in case of an error.
 
 The _tracefs_option_is_supported()_ and _tracefs_option_is_enabled()_ functions return true if the
 option in supported / enabled, or false otherwise.
@@ -52,14 +51,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 +65,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..f539323 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -211,56 +211,52 @@ 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;
+	unsigned long long set;
 	char file[PATH_MAX];
-	struct dirent *dent;
-	char *dname = NULL;
-	DIR *dir = NULL;
+	struct stat st;
 	long long val;
+	char *path;
+	int ret;
 
-	bitmask = calloc(1, sizeof(struct tracefs_options_mask));
-	if (!bitmask)
-		return NULL;
-	dname = tracefs_instance_get_file(instance, "options");
-	if (!dname)
-		goto error;
-	dir = opendir(dname);
-	if (!dir)
-		goto error;
-
-	while ((dent = readdir(dir))) {
-		if (*dent->d_name == '.')
-			continue;
-		if (enabled) {
-			snprintf(file, PATH_MAX, "options/%s", dent->d_name);
-			if (tracefs_instance_file_read_number(instance, file, &val) != 0 ||
-			    val != 1)
-				continue;
+	bitmask = enabled? enabled_opts_mask(instance) :
+			   supported_opts_mask(instance);
+
+	for (id = 1; id < TRACEFS_OPTION_MAX; id++) {
+		snprintf(file, PATH_MAX, "options/%s", options_map[id]);
+		path = tracefs_instance_get_file(instance, file);
+		if (!path)
+			return NULL;
+
+		set = 1;
+		ret = stat(path, &st);
+		if (ret < 0 || !S_ISREG(st.st_mode)) {
+			set = 0;
+		} else if (enabled) {
+			ret = tracefs_instance_file_read_number(instance, file, &val);
+			if (ret != 0 || val != 1)
+				set = 0;
 		}
-		id = tracefs_option_id(dent->d_name);
-		if (id > TRACEFS_OPTION_INVALID)
-			bitmask->mask |= (1ULL << (id - 1));
+
+		pthread_mutex_lock(lock);
+		bitmask->mask = (bitmask->mask & ~(1ULL << (id - 1))) | (set << (id - 1));
+		pthread_mutex_unlock(lock);
+
+		tracefs_put_tracing_file(path);
 	}
-	closedir(dir);
-	tracefs_put_tracing_file(dname);
 
-	return bitmask;
 
-error:
-	if (dir)
-		closedir(dir);
-	tracefs_put_tracing_file(dname);
-	free(bitmask);
-	return NULL;
+	return bitmask;
 }
 
 /**
  * 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 +267,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 +278,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 +366,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 5/5] libtracefs: Rename tracefs_option_is_set()
  2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
                   ` (3 preceding siblings ...)
  2021-04-09 18:04 ` [PATCH v4 4/5] libtracefs: Option's bit masks to be owned by the instance Yordan Karadzhov (VMware)
@ 2021-04-09 18:04 ` Yordan Karadzhov (VMware)
  2021-04-09 18:26 ` [PATCH v4 0/5] Refactor the APIs for tracing options Steven Rostedt
  5 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-04-09 18:04 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, tz.stoyanov, 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>
---
 Documentation/libtracefs-option-bits.txt | 10 +++++-----
 include/tracefs.h                        |  4 ++--
 src/tracefs-tools.c                      |  4 ++--
 utest/tracefs-utest.c                    |  8 ++++----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/libtracefs-option-bits.txt b/Documentation/libtracefs-option-bits.txt
index d713a5e..10e832e 100644
--- a/Documentation/libtracefs-option-bits.txt
+++ b/Documentation/libtracefs-option-bits.txt
@@ -3,7 +3,7 @@ libtracefs(3)
 
 NAME
 ----
-tracefs_option_set, tracefs_option_clear, tracefs_option_is_set -
+tracefs_option_set, tracefs_option_clear, tracefs_option_mask_is_set -
 Set, clear, check option in a bitmask.
 
 SYNOPSIS
@@ -14,7 +14,7 @@ SYNOPSIS
 
 void *tracefs_option_set*(struct tracefs_options_mask pass:[*]_options_, enum tracefs_option_id _id_);
 void *tracefs_option_clear*(struct tracefs_options_mask pass:[*]_options_, enum tracefs_option_id _id_);
-bool *tracefs_option_is_set*(struct tracefs_options_mask _options_, enum tracefs_option_id _id_);
+bool *tracefs_option_mask_is_set*(struct tracefs_options_mask _options_, enum tracefs_option_id _id_);
 --
 
 DESCRIPTION
@@ -27,12 +27,12 @@ _options_ bitmask.
 The _tracefs_option_clear()_ function clears the bit, corresponding to the option with _id_ in the
 _options_ bitmask.
 
-The _tracefs_option_is_set()_ function checks if the bit, corresponding to the option with _id_ is
+The _tracefs_option_mask_is_set()_ function checks if the bit, corresponding to the option with _id_ is
 set in the _options_ bitmask.
 
 RETURN VALUE
 ------------
-The _tracefs_option_is_set()_ function returns true if the bit is set, false otherwise.
+The _tracefs_option_mask_is_set()_ function returns true if the bit is set, false otherwise.
 
 EXAMPLE
 -------
@@ -45,7 +45,7 @@ memset(&options, 0, sizeof(options));
 ...
 tracefs_option_set(&options, TRACEFS_OPTION_EVENT_FORK | TRACEFS_OPTION_FUNCTION_FORK);
 ...
-if (tracefs_option_is_set(options, TRACEFS_OPTION_EVENT_FORK))
+if (tracefs_option_mask_is_set(options, TRACEFS_OPTION_EVENT_FORK))
 	tracefs_option_clear(&options, TRACEFS_OPTION_EVENT_FORK);
 ...
 --
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 f539323..15ad3b6 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -359,14 +359,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 0/5] Refactor the APIs for tracing options
  2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
                   ` (4 preceding siblings ...)
  2021-04-09 18:04 ` [PATCH v4 5/5] libtracefs: Rename tracefs_option_is_set() Yordan Karadzhov (VMware)
@ 2021-04-09 18:26 ` Steven Rostedt
  2021-04-09 18:26   ` Steven Rostedt
  5 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2021-04-09 18:26 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel, tz.stoyanov

On Fri,  9 Apr 2021 21:04:18 +0300
"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-bits.txt |  10 +--
>  Documentation/libtracefs-option-get.txt  |   7 +-
>  include/tracefs-local.h                  |  12 +++
>  include/tracefs.h                        |  11 +--
>  src/tracefs-instance.c                   |  15 ++++
>  src/tracefs-tools.c                      | 103 +++++++++--------------
>  utest/tracefs-utest.c                    |  12 +--
>  7 files changed, 81 insertions(+), 89 deletions(-)
> 

Bah, I just noticed this patch series after sending out my own.

OK, I'll use this one and see if it covers everything.

Thanks!

-- Steve


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 0/5] Refactor the APIs for tracing options
  2021-04-09 18:26 ` [PATCH v4 0/5] Refactor the APIs for tracing options Steven Rostedt
@ 2021-04-09 18:26   ` Steven Rostedt
  0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-04-09 18:26 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel, tz.stoyanov

On Fri,  9 Apr 2021 21:04:18 +0300
"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-bits.txt |  10 +--
>  Documentation/libtracefs-option-get.txt  |   7 +-
>  include/tracefs-local.h                  |  12 +++
>  include/tracefs.h                        |  11 +--
>  src/tracefs-instance.c                   |  15 ++++
>  src/tracefs-tools.c                      | 103 +++++++++--------------
>  utest/tracefs-utest.c                    |  12 +--
>  7 files changed, 81 insertions(+), 89 deletions(-)
> 

Bah, I just noticed this patch series after sending out my own updates to
the last one.

OK, I'll use this one and see if it covers everything.

Thanks!

-- Steve


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 4/5] libtracefs: Option's bit masks to be owned by the instance
  2021-04-09 18:04 ` [PATCH v4 4/5] libtracefs: Option's bit masks to be owned by the instance Yordan Karadzhov (VMware)
@ 2021-04-09 18:40   ` Steven Rostedt
  0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-04-09 18:40 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel, tz.stoyanov

On Fri,  9 Apr 2021 21:04:22 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +__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;
> +}

I'll fix this up, but for the future, there should be a space between the
condition and the "?".

	return instance ? &instance->supported_opts : &toplevel_supported_opts;

-- Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-04-09 18:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 18:04 [PATCH v4 0/5] Refactor the APIs for tracing options Yordan Karadzhov (VMware)
2021-04-09 18:04 ` [PATCH v4 1/5] libtracefs: Fix issues with tracefs_option_id() Yordan Karadzhov (VMware)
2021-04-09 18:04 ` [PATCH v4 2/5] libtracefs: Modify the arguments of tracefs_option_is_set() Yordan Karadzhov (VMware)
2021-04-09 18:04 ` [PATCH v4 3/5] libtracefs: Encapsulate "struct tracefs_options_mask" Yordan Karadzhov (VMware)
2021-04-09 18:04 ` [PATCH v4 4/5] libtracefs: Option's bit masks to be owned by the instance Yordan Karadzhov (VMware)
2021-04-09 18:40   ` Steven Rostedt
2021-04-09 18:04 ` [PATCH v4 5/5] libtracefs: Rename tracefs_option_is_set() Yordan Karadzhov (VMware)
2021-04-09 18:26 ` [PATCH v4 0/5] Refactor the APIs for tracing options Steven Rostedt
2021-04-09 18:26   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).