Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
       [not found] <20190910084707.18380-1-sakari.ailus@linux.intel.com>
@ 2019-09-10  8:46 ` Sakari Ailus
  2019-09-10  9:22   ` Andy Shevchenko
  2019-09-10 11:18   ` Steven Rostedt
  0 siblings, 2 replies; 11+ messages in thread
From: Sakari Ailus @ 2019-09-10  8:46 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
	Heikki Krogerus, Joe Perches, Steven Rostedt,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

There are no in-kernel %p[fF] users left. Convert the traceevent tool,
too, to align with the kernel.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Cc: linux-trace-devel@vger.kernel.org
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 .../Documentation/libtraceevent-func_apis.txt          | 10 +++++-----
 tools/lib/traceevent/event-parse.c                     |  7 +++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
index 38bfea30a5f64..f6aca0df2151a 100644
--- a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
+++ b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
@@ -59,12 +59,12 @@ parser context.
 
 The _tep_register_function()_ function registers a function name mapped to an
 address and (optional) module. This mapping is used in case the function tracer
-or events have "%pF" or "%pS" parameter in its format string. It is common to
-pass in the kallsyms function names with their corresponding addresses with this
+or events have "%pS" parameter in its format string. It is common to pass in
+the kallsyms function names with their corresponding addresses with this
 function. The _tep_ argument is the trace event parser context. The _name_ is
-the name of the function, the string is copied internally. The _addr_ is
-the start address of the function. The _mod_ is the kernel module
-the function may be in (NULL for none).
+the name of the function, the string is copied internally. The _addr_ is the
+start address of the function. The _mod_ is the kernel module the function may
+be in (NULL for none).
 
 The _tep_register_print_string()_ function  registers a string by the address
 it was stored in the kernel. Some strings internal to the kernel with static
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index b36b536a9fcba..1d7927ff32660 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4335,8 +4335,6 @@ static struct tep_print_arg *make_bprint_args(char *fmt, void *data, int size, s
 					switch (*ptr) {
 					case 's':
 					case 'S':
-					case 'f':
-					case 'F':
 					case 'x':
 						break;
 					default:
@@ -4455,12 +4453,13 @@ get_bprint_format(void *data, int size __maybe_unused,
 
 	printk = find_printk(tep, addr);
 	if (!printk) {
-		if (asprintf(&format, "%%pf: (NO FORMAT FOUND at %llx)\n", addr) < 0)
+		if (asprintf(&format, "%%ps: (NO FORMAT FOUND at %llx)\n",
+			     addr) < 0)
 			return NULL;
 		return format;
 	}
 
-	if (asprintf(&format, "%s: %s", "%pf", printk->printk) < 0)
+	if (asprintf(&format, "%s: %s", "%ps", printk->printk) < 0)
 		return NULL;
 
 	return format;
-- 
2.20.1


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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10  8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
@ 2019-09-10  9:22   ` Andy Shevchenko
  2019-09-10 11:18   ` Steven Rostedt
  1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2019-09-10  9:22 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, linux-acpi, devicetree,
	Rob Herring, Heikki Krogerus, Joe Perches, Steven Rostedt,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, Sep 10, 2019 at 11:46:56AM +0300, Sakari Ailus wrote:
> There are no in-kernel %p[fF] users left. Convert the traceevent tool,
> too, to align with the kernel.

>  function. The _tep_ argument is the trace event parser context. The _name_ is
> -the name of the function, the string is copied internally. The _addr_ is
> -the start address of the function. The _mod_ is the kernel module
> -the function may be in (NULL for none).
> +the name of the function, the string is copied internally. The _addr_ is the
> +start address of the function. The _mod_ is the kernel module the function may
> +be in (NULL for none).

This seems a churn of reformatting.

> -		if (asprintf(&format, "%%pf: (NO FORMAT FOUND at %llx)\n", addr) < 0)
> +		if (asprintf(&format, "%%ps: (NO FORMAT FOUND at %llx)\n",
> +			     addr) < 0)

Splitting line also seems a churn to me. But this one is up to maintainers.


Other than above, looks good to me, thanks!

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10  8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
  2019-09-10  9:22   ` Andy Shevchenko
@ 2019-09-10 11:18   ` Steven Rostedt
  2019-09-10 17:18     ` Joe Perches
  1 sibling, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2019-09-10 11:18 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
	devicetree, Rob Herring, Heikki Krogerus, Joe Perches,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, 10 Sep 2019 11:46:56 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> wrote:

> There are no in-kernel %p[fF] users left. Convert the traceevent tool,
> too, to align with the kernel.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> Cc: linux-trace-devel@vger.kernel.org
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
>  .../Documentation/libtraceevent-func_apis.txt          | 10 +++++-----
>  tools/lib/traceevent/event-parse.c                     |  7 +++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
> index 38bfea30a5f64..f6aca0df2151a 100644
> --- a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
> +++ b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
> @@ -59,12 +59,12 @@ parser context.
>  
>  The _tep_register_function()_ function registers a function name mapped to an
>  address and (optional) module. This mapping is used in case the function tracer
> -or events have "%pF" or "%pS" parameter in its format string. It is common to
> -pass in the kallsyms function names with their corresponding addresses with this
> +or events have "%pS" parameter in its format string. It is common to pass in
> +the kallsyms function names with their corresponding addresses with this
>  function. The _tep_ argument is the trace event parser context. The _name_ is
> -the name of the function, the string is copied internally. The _addr_ is
> -the start address of the function. The _mod_ is the kernel module
> -the function may be in (NULL for none).
> +the name of the function, the string is copied internally. The _addr_ is the
> +start address of the function. The _mod_ is the kernel module the function may
> +be in (NULL for none).
>  
>  The _tep_register_print_string()_ function  registers a string by the address
>  it was stored in the kernel. Some strings internal to the kernel with static
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index b36b536a9fcba..1d7927ff32660 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -4335,8 +4335,6 @@ static struct tep_print_arg *make_bprint_args(char *fmt, void *data, int size, s
>  					switch (*ptr) {
>  					case 's':
>  					case 'S':
> -					case 'f':
> -					case 'F':

This file is used to parse output from older kernels, so remove this hunk.

It's not just for the lastest kernel. We must maintain backward
compatibility here too. If there use to be a usage of this, then we
must keep it until the kernels are no longer used (perhaps 7 years?)


>  					case 'x':
>  						break;
>  					default:
> @@ -4455,12 +4453,13 @@ get_bprint_format(void *data, int size
> __maybe_unused, 
>  	printk = find_printk(tep, addr);
>  	if (!printk) {
> -		if (asprintf(&format, "%%pf: (NO FORMAT FOUND at
> %llx)\n", addr) < 0)
> +		if (asprintf(&format, "%%ps: (NO FORMAT FOUND at
> %llx)\n",
> +			     addr) < 0)

Remove the line break. I hate the 80 character limit especially when it
makes the code look worse. Like it does here.

-- Steve

>  			return NULL;
>  		return format;
>  	}
>  
> -	if (asprintf(&format, "%s: %s", "%pf", printk->printk) < 0)
> +	if (asprintf(&format, "%s: %s", "%ps", printk->printk) < 0)
>  		return NULL;
>  
>  	return format;


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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10 11:18   ` Steven Rostedt
@ 2019-09-10 17:18     ` Joe Perches
  2019-09-10 18:26       ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2019-09-10 17:18 UTC (permalink / raw)
  To: Steven Rostedt, Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
	devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, 2019-09-10 at 07:18 -0400, Steven Rostedt wrote:
> On Tue, 10 Sep 2019 11:46:56 +0300
> Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> 
> > There are no in-kernel %p[fF] users left. Convert the traceevent tool,
> > too, to align with the kernel.
[]
> > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
[]
> > @@ -4335,8 +4335,6 @@ static struct tep_print_arg *make_bprint_args(char *fmt, void *data, int size, s
> >  					switch (*ptr) {
> >  					case 's':
> >  					case 'S':
> > -					case 'f':
> > -					case 'F':
> 
> This file is used to parse output from older kernels, so remove this hunk.
> 
> It's not just for the lastest kernel. We must maintain backward
> compatibility here too. If there use to be a usage of this, then we
> must keep it until the kernels are no longer used (perhaps 7 years?)

That argues for not using "%pfw" at all for some number of years.

Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'




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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10 17:18     ` Joe Perches
@ 2019-09-10 18:26       ` Steven Rostedt
  2019-09-10 18:42         ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2019-09-10 18:26 UTC (permalink / raw)
  To: Joe Perches
  Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
	linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, 10 Sep 2019 10:18:44 -0700
Joe Perches <joe@perches.com> wrote:

> > It's not just for the lastest kernel. We must maintain backward
> > compatibility here too. If there use to be a usage of this, then we
> > must keep it until the kernels are no longer used (perhaps 7 years?)  
> 
> That argues for not using "%pfw" at all for some number of years.
> 
> Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'

  -ENOCOMPREHENSION

-- Steve

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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10 18:26       ` Steven Rostedt
@ 2019-09-10 18:42         ` Joe Perches
  2019-09-10 19:03           ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2019-09-10 18:42 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
	linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, 2019-09-10 at 14:26 -0400, Steven Rostedt wrote:
> On Tue, 10 Sep 2019 10:18:44 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > > It's not just for the lastest kernel. We must maintain backward
> > > compatibility here too. If there use to be a usage of this, then we
> > > must keep it until the kernels are no longer used (perhaps 7 years?)  
> > 
> > That argues for not using "%pfw" at all for some number of years.
> > 
> > Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'
>
>   -ENOCOMPREHENSION

Perhaps you were not copied on the whole series.

https://lore.kernel.org/lkml/20190910084707.18380-1-sakari.ailus@linux.intel.com/

As I understand it, Sakair Ailus is proposing to
obsolete the current vsprintf "%p[Ff]" extension
and replace the usage with a new "%pfw" extension
which would emit the name of a pointer to "struct fwnode {}".

https://lore.kernel.org/lkml/20190910084707.18380-10-sakari.ailus@linux.intel.com/

If reusing "%pf<foo>" is a problem, then instead
it might be reasonable to have a new "%pn<foo>" for
that use instead.

btw:

Is there kernel version information available in
trace output files?

If so, it might be reasonable to change the tooling
there instead.



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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10 18:42         ` Joe Perches
@ 2019-09-10 19:03           ` Steven Rostedt
  2019-09-10 19:44             ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2019-09-10 19:03 UTC (permalink / raw)
  To: Joe Perches
  Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
	linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, 10 Sep 2019 11:42:06 -0700
Joe Perches <joe@perches.com> wrote:

> On Tue, 2019-09-10 at 14:26 -0400, Steven Rostedt wrote:
> > On Tue, 10 Sep 2019 10:18:44 -0700
> > Joe Perches <joe@perches.com> wrote:
> >   
> > > > It's not just for the lastest kernel. We must maintain backward
> > > > compatibility here too. If there use to be a usage of this, then we
> > > > must keep it until the kernels are no longer used (perhaps 7 years?)    
> > > 
> > > That argues for not using "%pfw" at all for some number of years.
> > > 
> > > Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'  
> >
> >   -ENOCOMPREHENSION  
> 
> Perhaps you were not copied on the whole series.
> 
> https://lore.kernel.org/lkml/20190910084707.18380-1-sakari.ailus@linux.intel.com/

Thanks for the link.

> 
> As I understand it, Sakair Ailus is proposing to
> obsolete the current vsprintf "%p[Ff]" extension
> and replace the usage with a new "%pfw" extension
> which would emit the name of a pointer to "struct fwnode {}".
> 
> https://lore.kernel.org/lkml/20190910084707.18380-10-sakari.ailus@linux.intel.com/
> 
> If reusing "%pf<foo>" is a problem, then instead
> it might be reasonable to have a new "%pn<foo>" for
> that use instead.
> 
> btw:
> 
> Is there kernel version information available in
> trace output files?

Not really. This is just a library that parses the trace event formats,
there's not kernel versions passed in, but we do use variations in
formats and such to determine what is supported.

> 
> If so, it might be reasonable to change the tooling
> there instead.
> 

Actually, I think we could just look to see if "%pfw" is used and fall
to that, otherwise consider it an older kernel and do it the original
way.

-- Steve


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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10 19:03           ` Steven Rostedt
@ 2019-09-10 19:44             ` Joe Perches
  2019-09-16 11:41               ` Sakari Ailus
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2019-09-10 19:44 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
	linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Tue, 2019-09-10 at 15:03 -0400, Steven Rostedt wrote:
> On Tue, 10 Sep 2019 11:42:06 -0700
[]
> > btw:
> > 
> > Is there kernel version information available in
> > trace output files?
> 
> Not really. This is just a library that parses the trace event formats,
> there's not kernel versions passed in, but we do use variations in
> formats and such to determine what is supported.
> 
> > If so, it might be reasonable to change the tooling
> > there instead.
> > 
> 
> Actually, I think we could just look to see if "%pfw" is used and fall
> to that, otherwise consider it an older kernel and do it the original
> way.

Well, if you think that works, OK great.

But could that work?
How would an individual trace record know if
another trace record used %pfw?

Perhaps not reusing %pf, marking it reserved
for a period of years, and using another unused
prefix %p<type> like %pnfw may be simpler.



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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-10 19:44             ` Joe Perches
@ 2019-09-16 11:41               ` Sakari Ailus
  2019-09-16 14:37                 ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Sakari Ailus @ 2019-09-16 11:41 UTC (permalink / raw)
  To: Joe Perches
  Cc: Steven Rostedt, Petr Mladek, linux-kernel, rafael,
	Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
	Heikki Krogerus, Arnaldo Carvalho de Melo, Tzvetomir Stoyanov,
	linux-trace-devel, Jiri Olsa, Namhyung Kim

Hi Joe, Steven,

On Tue, Sep 10, 2019 at 12:44:03PM -0700, Joe Perches wrote:
> On Tue, 2019-09-10 at 15:03 -0400, Steven Rostedt wrote:
> > On Tue, 10 Sep 2019 11:42:06 -0700
> []
> > > btw:
> > > 
> > > Is there kernel version information available in
> > > trace output files?
> > 
> > Not really. This is just a library that parses the trace event formats,
> > there's not kernel versions passed in, but we do use variations in
> > formats and such to determine what is supported.
> > 
> > > If so, it might be reasonable to change the tooling
> > > there instead.
> > > 
> > 
> > Actually, I think we could just look to see if "%pfw" is used and fall
> > to that, otherwise consider it an older kernel and do it the original
> > way.
> 
> Well, if you think that works, OK great.
> 
> But could that work?
> How would an individual trace record know if
> another trace record used %pfw?
> 
> Perhaps not reusing %pf, marking it reserved
> for a period of years, and using another unused
> prefix %p<type> like %pnfw may be simpler.

%p[Ff]w does not exist (I grepped for it) in older kernels since v3.0. So
kernel support for %p[fF] and %pfw are mutually exclusive. If you're ok
with that, I could change the patch to check %pf isn't followed by 'w',
in order to support %pf on older kernels.

Although that still does not address using older tooling on newer kernels
with support for %pfw.

If you think that's an issue, I'll opt for another extension than %pfw,
which I chose originally since it's memorable --- fw for fwnode (names,
paths, and probably more in the future).

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-16 11:41               ` Sakari Ailus
@ 2019-09-16 14:37                 ` Steven Rostedt
  2019-09-18 13:08                   ` Sakari Ailus
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2019-09-16 14:37 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Joe Perches, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
	linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

On Mon, 16 Sep 2019 14:41:59 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> wrote:

> > Well, if you think that works, OK great.
> > 
> > But could that work?
> > How would an individual trace record know if
> > another trace record used %pfw?
> > 
> > Perhaps not reusing %pf, marking it reserved
> > for a period of years, and using another unused
> > prefix %p<type> like %pnfw may be simpler.  
> 
> %p[Ff]w does not exist (I grepped for it) in older kernels since v3.0. So
> kernel support for %p[fF] and %pfw are mutually exclusive. If you're ok
> with that, I could change the patch to check %pf isn't followed by 'w',
> in order to support %pf on older kernels.

I think that's what I suggested to do.

> 
> Although that still does not address using older tooling on newer kernels
> with support for %pfw.

That should be fine. I don't think it will crash those tools, they will
just give out wrong information, and if people complain, we can try to
get them to use the newer version of those tools ;-) (hopefully they
don't complain to Linus).

> 
> If you think that's an issue, I'll opt for another extension than %pfw,
> which I chose originally since it's memorable --- fw for fwnode (names,
> paths, and probably more in the future).
> 

I'm fine with the switch, as long as newer tools know how to handle it.

Make sure we also add a comment in the Linux kernel code that states
that older kernels use to have 'f' and 'F' and that new tools look for
'fw' to denote that this isn't an older kernel. This way, people will
hopefully not add another 'fX' pointer name.

-- Steve

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

* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
  2019-09-16 14:37                 ` Steven Rostedt
@ 2019-09-18 13:08                   ` Sakari Ailus
  0 siblings, 0 replies; 11+ messages in thread
From: Sakari Ailus @ 2019-09-18 13:08 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Joe Perches, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
	linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
	Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
	Jiri Olsa, Namhyung Kim

Hi Steven,

On Mon, Sep 16, 2019 at 10:37:55AM -0400, Steven Rostedt wrote:
> > If you think that's an issue, I'll opt for another extension than %pfw,
> > which I chose originally since it's memorable --- fw for fwnode (names,
> > paths, and probably more in the future).
> > 
> 
> I'm fine with the switch, as long as newer tools know how to handle it.
> 
> Make sure we also add a comment in the Linux kernel code that states
> that older kernels use to have 'f' and 'F' and that new tools look for
> 'fw' to denote that this isn't an older kernel. This way, people will
> hopefully not add another 'fX' pointer name.

Good point. I'll add a comment on this to make_bprint_args() in
tools/lib/traceevent/event-parse.c as well as in vsprintf.c.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190910084707.18380-1-sakari.ailus@linux.intel.com>
2019-09-10  8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
2019-09-10  9:22   ` Andy Shevchenko
2019-09-10 11:18   ` Steven Rostedt
2019-09-10 17:18     ` Joe Perches
2019-09-10 18:26       ` Steven Rostedt
2019-09-10 18:42         ` Joe Perches
2019-09-10 19:03           ` Steven Rostedt
2019-09-10 19:44             ` Joe Perches
2019-09-16 11:41               ` Sakari Ailus
2019-09-16 14:37                 ` Steven Rostedt
2019-09-18 13:08                   ` Sakari Ailus

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git