* [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() @ 2022-11-28 13:21 Bean Huo 2022-11-28 15:24 ` Steven Rostedt 0 siblings, 1 reply; 7+ messages in thread From: Bean Huo @ 2022-11-28 13:21 UTC (permalink / raw) To: linux-trace-devel, rostedt, tz.stoyanov; +Cc: Bean Huo From: Bean Huo <beanhuo@micron.com> If it eventually mounts successfully, it should return 1 instead of 0, otherwise it will make the caller's verification logic more complicated Signed-off-by: Bean Huo <beanhuo@micron.com> --- src/tracefs-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracefs-utils.c b/src/tracefs-utils.c index 777912e..5d6e977 100644 --- a/src/tracefs-utils.c +++ b/src/tracefs-utils.c @@ -188,7 +188,7 @@ int tracefs_tracing_dir_is_mounted(bool mount, const char **path) return -1; if (path) *path = dir; - return 0; + return 1; } /** -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() 2022-11-28 13:21 [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() Bean Huo @ 2022-11-28 15:24 ` Steven Rostedt 2022-11-28 15:41 ` Bean Huo 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2022-11-28 15:24 UTC (permalink / raw) To: Bean Huo; +Cc: linux-trace-devel, tz.stoyanov, Bean Huo On Mon, 28 Nov 2022 14:21:06 +0100 Bean Huo <beanhuo@iokpp.de> wrote: > From: Bean Huo <beanhuo@micron.com> > > If it eventually mounts successfully, it should return 1 instead of > 0, otherwise it will make the caller's verification logic more complicated The man page shows: The tracefs_tracing_dir_is_mounted() returns 1 if the tracing directory is already mounted, 0 if it is not, and -1 on error. If you only want to mount it and not care if it was already mounted then use tracefs_tracing_dir(), as it will return the path of the mount point and try to mount it if it is not already, or NULL if it could not mount it. This function is specifically created to tell the application if it mounted or not, so that it could unmount it when it is done. We have applications that do this. So, no, I'm not taking this change. It breaks the use case for this function. -- Steve > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > src/tracefs-utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/tracefs-utils.c b/src/tracefs-utils.c > index 777912e..5d6e977 100644 > --- a/src/tracefs-utils.c > +++ b/src/tracefs-utils.c > @@ -188,7 +188,7 @@ int tracefs_tracing_dir_is_mounted(bool mount, const char **path) > return -1; > if (path) > *path = dir; > - return 0; > + return 1; > } > > /** ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() 2022-11-28 15:24 ` Steven Rostedt @ 2022-11-28 15:41 ` Bean Huo 2022-11-28 16:29 ` Bean Huo 2022-11-28 18:37 ` Steven Rostedt 0 siblings, 2 replies; 7+ messages in thread From: Bean Huo @ 2022-11-28 15:41 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel, tz.stoyanov, Bean Huo On Mon, 2022-11-28 at 10:24 -0500, Steven Rostedt wrote: > On Mon, 28 Nov 2022 14:21:06 +0100 > > Bean Huo <beanhuo@iokpp.de> wrote: > > > > > From: Bean Huo <beanhuo@micron.com> > > If it eventually mounts successfully, it should return 1 instead of > > 0, otherwise it will make the caller's verification logic more > > complicated > > > The man page shows: > > > > The tracefs_tracing_dir_is_mounted() returns 1 if the tracing > directory is > > already mounted, 0 if it is not, and -1 on error. > > > > If you only want to mount it and not care if it was already mounted > then > > use tracefs_tracing_dir(), as it will return the path of the mount > point > > and try to mount it if it is not already, or NULL if it could not > mount it. > > > Hi Steve, I used to reply on tracefs_tracing_dir(). But after updated the libtracefs yesterday, I found my app doesn't work as before, then checked the changelog, and switch to use tracefs_tracing_dir_is_mounted. in your latest libtracefs: const char *tracefs_tracing_dir(void) { ... tracing_dir = trace_find_tracing_dir(false);//shold be changed to true? ... } __hidden char *trace_find_tracing_dir(bool debugfs) { return find_tracing_dir(debugfs, false); } static char *find_tracing_dir(bool debugfs, bool mount) { ... if (!mount || mount_debugfs() < 0) return NULL; ... } Kind regards, Bean ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() 2022-11-28 15:41 ` Bean Huo @ 2022-11-28 16:29 ` Bean Huo 2022-11-28 18:37 ` Steven Rostedt 1 sibling, 0 replies; 7+ messages in thread From: Bean Huo @ 2022-11-28 16:29 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel, tz.stoyanov, Bean Huo On Mon, 2022-11-28 at 16:41 +0100, Bean Huo wrote: > On Mon, 2022-11-28 at 10:24 -0500, Steven Rostedt wrote: > > On Mon, 28 Nov 2022 14:21:06 +0100 > > > > Bean Huo <beanhuo@iokpp.de> wrote: > > > > > > > > > From: Bean Huo <beanhuo@micron.com> > > > If it eventually mounts successfully, it should return 1 instead > > > of > > > 0, otherwise it will make the caller's verification logic more > > > complicated > > > > The man page shows: > > > > > > > > The tracefs_tracing_dir_is_mounted() returns 1 if the tracing > > directory is > > > > already mounted, 0 if it is not, and -1 on error. > > > > > > > > If you only want to mount it and not care if it was already mounted > > then > > > > use tracefs_tracing_dir(), as it will return the path of the mount > > point > > > > and try to mount it if it is not already, or NULL if it could not > > mount it. > > > > > > > > Hi Steve, > > I used to reply on tracefs_tracing_dir(). But after updated the > libtracefs yesterday, I found my app doesn't work as before, then > checked the changelog, and switch to use > tracefs_tracing_dir_is_mounted. > > in your latest libtracefs: > > > const char *tracefs_tracing_dir(void) { > > ... > tracing_dir = trace_find_tracing_dir(false);//shold be changed to > true? > ... > > } > > > __hidden char *trace_find_tracing_dir(bool debugfs) > { > return find_tracing_dir(debugfs, false); > } > > > > static char *find_tracing_dir(bool debugfs, bool mount) { > > ... > if (!mount || mount_debugfs() < 0) > return NULL; > > > ... > > > } > > instead of changing tracefs_tracing_dir_is_mounted(), or we could change trace_find_tracing_dir(), let it mount the tracefs in case of it is not mounted when we call tracefs_tracing_dir(): --- a/src/tracefs-utils.c +++ b/src/tracefs-utils.c @@ -200,7 +200,7 @@ int tracefs_tracing_dir_is_mounted(bool mount, const char **path) */ __hidden char *trace_find_tracing_dir(bool debugfs) { - return find_tracing_dir(debugfs, false); + return find_tracing_dir(debugfs, true); } Kind regard, Bean ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() 2022-11-28 15:41 ` Bean Huo 2022-11-28 16:29 ` Bean Huo @ 2022-11-28 18:37 ` Steven Rostedt 2022-12-06 21:13 ` Steven Rostedt 1 sibling, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2022-11-28 18:37 UTC (permalink / raw) To: Bean Huo; +Cc: linux-trace-devel, tz.stoyanov, Bean Huo On Mon, 28 Nov 2022 16:41:54 +0100 Bean Huo <beanhuo@iokpp.de> wrote: > I used to reply on tracefs_tracing_dir(). But after updated the > libtracefs yesterday, I found my app doesn't work as before, then > checked the changelog, and switch to use > tracefs_tracing_dir_is_mounted. OK, so this is a regression and needs to be fixed. > > in your latest libtracefs: > > > const char *tracefs_tracing_dir(void) { > > ... > tracing_dir = trace_find_tracing_dir(false);//shold be changed to true? > ... Ah, I think I see the issue here. If it returns NULL, then we need to do more. I'll take a look into this later today. Thanks for reporting this. -- Steve > > } > > > __hidden char *trace_find_tracing_dir(bool debugfs) > { > return find_tracing_dir(debugfs, false); > } > > > > static char *find_tracing_dir(bool debugfs, bool mount) { > > ... > if (!mount || mount_debugfs() < 0) > return NULL; > > > ... > > > } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() 2022-11-28 18:37 ` Steven Rostedt @ 2022-12-06 21:13 ` Steven Rostedt 2022-12-08 12:05 ` Bean Huo 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2022-12-06 21:13 UTC (permalink / raw) To: Bean Huo; +Cc: linux-trace-devel, tz.stoyanov, Bean Huo On Mon, 28 Nov 2022 13:37:15 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > Ah, I think I see the issue here. If it returns NULL, then we need to do > more. I'll take a look into this later today. Can you test this patch? Thanks, -- Steve diff --git a/src/tracefs-utils.c b/src/tracefs-utils.c index 777912e46821..d91ff40eee87 100644 --- a/src/tracefs-utils.c +++ b/src/tracefs-utils.c @@ -245,7 +245,7 @@ const char *tracefs_tracing_dir(void) if (tracing_dir) return tracing_dir; - tracing_dir = trace_find_tracing_dir(false); + tracing_dir = find_tracing_dir(false, true); return tracing_dir; } @@ -263,7 +263,7 @@ const char *tracefs_debug_dir(void) if (debug_dir) return debug_dir; - debug_dir = trace_find_tracing_dir(true); + debug_dir = find_tracing_dir(true, true); return debug_dir; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() 2022-12-06 21:13 ` Steven Rostedt @ 2022-12-08 12:05 ` Bean Huo 0 siblings, 0 replies; 7+ messages in thread From: Bean Huo @ 2022-12-08 12:05 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel, tz.stoyanov, Bean Huo On Tue, 2022-12-06 at 16:13 -0500, Steven Rostedt wrote: > On Mon, 28 Nov 2022 13:37:15 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > Ah, I think I see the issue here. If it returns NULL, then we need > > to do > > more. I'll take a look into this later today. > > Can you test this patch? > > Thanks, > > -- Steve > > diff --git a/src/tracefs-utils.c b/src/tracefs-utils.c > index 777912e46821..d91ff40eee87 100644 > --- a/src/tracefs-utils.c > +++ b/src/tracefs-utils.c > @@ -245,7 +245,7 @@ const char *tracefs_tracing_dir(void) > if (tracing_dir) > return tracing_dir; > > - tracing_dir = trace_find_tracing_dir(false); > + tracing_dir = find_tracing_dir(false, true); > return tracing_dir; > } > > @@ -263,7 +263,7 @@ const char *tracefs_debug_dir(void) > if (debug_dir) > return debug_dir; > > - debug_dir = trace_find_tracing_dir(true); > + debug_dir = find_tracing_dir(true, true); > return debug_dir; > } > yes, it also works, thanks! Kind regards, Bean ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-08 12:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-28 13:21 [PATCH] libtracefs: Fix wrong return value in tracefs_tracing_dir_is_mounted() Bean Huo 2022-11-28 15:24 ` Steven Rostedt 2022-11-28 15:41 ` Bean Huo 2022-11-28 16:29 ` Bean Huo 2022-11-28 18:37 ` Steven Rostedt 2022-12-06 21:13 ` Steven Rostedt 2022-12-08 12:05 ` Bean Huo
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).