All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Philippe Duplessis-Guindon <pduplessis@efficios.com>
Cc: linux-trace-devel@vger.kernel.org, rostedt <rostedt@goodmis.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [RFC PATCH] regmap: remove duplicate `type` field from `regcache_sync` trace event
Date: Mon, 23 Nov 2020 11:39:30 -0500 (EST)	[thread overview]
Message-ID: <725176746.55143.1606149570341.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20201123161519.4643-1-pduplessis@efficios.com>

Hi Philippe,

Some additional feedback:

The patch title could be changed to:

"tracing: Remove duplicate `type` field from regmap `regcache_sync` trace event"

to clarify that this belongs to tracing.

----- On Nov 23, 2020, at 11:15 AM, Philippe Duplessis-Guindon pduplessis@efficios.com wrote:

> I had an error saying that `regcache_sync` had 2 fields named `type` while using
> libtraceevent. This was the format of this event:

Please use the present rather than past when writing a commit message.

> 
>	$ sudo cat /sys/kernel/debug/tracing/events/regmap/regcache_sync/format
>	name: regcache_sync
>	ID: 1216
>	format:
>		field:unsigned short common_type;	offset:0;	size:2; signed:0;
>		field:unsigned char common_flags;	offset:2;	size:1; signed:0;
>		field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
>		field:int common_pid;	offset:4;	size:4;	signed:1;
> 
>		field:__data_loc char[] name;	offset:8;	size:4;	signed:1;
>		field:__data_loc char[] status;	offset:12;	size:4;	signed:1;
>		field:__data_loc char[] type;	offset:16;	size:4;	signed:1;
>		field:int type;    offset:20;    size:4;    signed:1;
> 
>	print fmt: "%s type=%s status=%s", __get_str(name), __get_str(type),
>	__get_str(status)
> 
> Erase the `int field` type, who was not assigned. This field was introduce by

who -> which

was introduce -> was introduced

Ideally you should use git blame to identify which commit introduced this issue.
e.g.:

git blame drivers/base/regmap/trace.h
[...]
593600890110c include/trace/events/regmap.h (Dimitris Papastamos 2011-09-19 14:34:04 +0100 134)                 __assign_str(status, status);
593600890110c include/trace/events/regmap.h (Dimitris Papastamos 2011-09-19 14:34:04 +0100 135)                 __assign_str(type, type);

commit 593600890110c02eb471cf844649dee213870416
Author: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
Date:   Mon Sep 19 14:34:04 2011 +0100

    regmap: Add the regcache_sync trace event

This information can be added to the commit message as:

Fixes commit 593600890110c ("regmap: Add the regcache_sync trace event")

Thanks,

Mathieu

> mistake and this commit removes it.
> 
> This is the format of this event after the fix:
> 
>	$ sudo cat /sys/kernel/debug/tracing/events/regmap/regcache_sync/format
>	name: regcache_sync
>	ID: 1266
>	format:
>		field:unsigned short common_type;	offset:0;	size:2;	signed:0;
>		field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
>		field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
>		field:int common_pid;	offset:4;	size:4;	signed:1;
> 
>		field:__data_loc char[] name;	offset:8;	size:4;	signed:1;
>		field:__data_loc char[] status;	offset:12;	size:4;	signed:1;
>		field:__data_loc char[] type;	offset:16;	size:4;	signed:1;
> 
>	print fmt: "%s type=%s status=%s", __get_str(name), __get_str(type),
>	__get_str(status)
> 
> Signed-off-by: Philippe Duplessis-Guindon <pduplessis@efficios.com>
> ---
> drivers/base/regmap/trace.h | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/base/regmap/trace.h b/drivers/base/regmap/trace.h
> index d4066fa079ab..9abee14df9ee 100644
> --- a/drivers/base/regmap/trace.h
> +++ b/drivers/base/regmap/trace.h
> @@ -126,7 +126,6 @@ TRACE_EVENT(regcache_sync,
> 		__string(       name,           regmap_name(map)	)
> 		__string(	status,		status			)
> 		__string(	type,		type			)
> -		__field(	int,		type			)
> 	),
> 
> 	TP_fast_assign(
> --
> 2.25.1

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2020-11-23 16:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23 16:15 [RFC PATCH] regmap: remove duplicate `type` field from `regcache_sync` trace event Philippe Duplessis-Guindon
2020-11-23 16:39 ` Mathieu Desnoyers [this message]
2020-11-23 18:10 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=725176746.55143.1606149570341.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=pduplessis@efficios.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.