All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vasily Averin <vvs-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
To: Roman Gushchin <roman.gushchin-fxUVXftIFDnyG1zEObXtfA@public.gmane.org>
Cc: Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org>,
	Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	kernel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>,
	Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Dennis Zhou <dennis-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
Subject: Re: [PATCH v2] percpu: improve percpu_alloc_percpu event trace
Date: Wed, 11 May 2022 08:11:54 +0300	[thread overview]
Message-ID: <30a47b4e-7c4b-cd2d-998d-cfaf8d12d342@openvz.org> (raw)
In-Reply-To: <YnsgbXKiNNSF+1ZO@carbon>

On 5/11/22 05:33, Roman Gushchin wrote:
> On Fri, May 06, 2022 at 10:29:25PM +0300, Vasily Averin wrote:
>>  TRACE_EVENT(percpu_alloc_percpu,
>>  
>> -	TP_PROTO(bool reserved, bool is_atomic, size_t size,
>> -		 size_t align, void *base_addr, int off, void __percpu *ptr),
>> +	TP_PROTO(unsigned long call_site,
>> +		 bool reserved, bool is_atomic, size_t size,
>> +		 size_t align, void *base_addr, int off,
>> +		 void __percpu *ptr, size_t bytes_alloc, gfp_t gfp_flags),
> 
> Don't we want to preserve the order and add the call_site at the end?
> Trace events are not ABI, but if we don't have a strong reason to break it,
> I'd preserve the old order.

I checked recent trace patches and found that order changes is acceptable.

commit 8c39b8bc82aafcc8dd378bd79c76fac8e8a89c8d
Author: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date:   Fri Jan 14 11:44:54 2022 +0000

    cachefiles: Make some tracepoint adjustments

-           TP_printk("o=%08x i=%lx e=%d",
-                     __entry->obj, __entry->ino, __entry->error)
+           TP_printk("o=%08x dB=%lx B=%lx e=%d",
+                     __entry->obj, __entry->dino, __entry->ino, __entry->error)

On the other hand I'm agree to keep old order by default.
that's why I added bytes_alloc and gfp_flags to end of output.
However I think call_site is an exception. In all cases found, 
call_site is output first.
For me personally it simplified output parsing.

So I would like to know Steven's position on this question.

Thank you,
	Vasily Averin

WARNING: multiple messages have this Message-ID (diff)
From: Vasily Averin <vvs@openvz.org>
To: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Shakeel Butt <shakeelb@google.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	kernel@openvz.org, linux-kernel@vger.kernel.org,
	Vlastimil Babka <vbabka@suse.cz>, Michal Hocko <mhocko@suse.com>,
	cgroups@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Dennis Zhou <dennis@kernel.org>, Tejun Heo <tj@kernel.org>,
	Christoph Lameter <cl@linux.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH v2] percpu: improve percpu_alloc_percpu event trace
Date: Wed, 11 May 2022 08:11:54 +0300	[thread overview]
Message-ID: <30a47b4e-7c4b-cd2d-998d-cfaf8d12d342@openvz.org> (raw)
In-Reply-To: <YnsgbXKiNNSF+1ZO@carbon>

On 5/11/22 05:33, Roman Gushchin wrote:
> On Fri, May 06, 2022 at 10:29:25PM +0300, Vasily Averin wrote:
>>  TRACE_EVENT(percpu_alloc_percpu,
>>  
>> -	TP_PROTO(bool reserved, bool is_atomic, size_t size,
>> -		 size_t align, void *base_addr, int off, void __percpu *ptr),
>> +	TP_PROTO(unsigned long call_site,
>> +		 bool reserved, bool is_atomic, size_t size,
>> +		 size_t align, void *base_addr, int off,
>> +		 void __percpu *ptr, size_t bytes_alloc, gfp_t gfp_flags),
> 
> Don't we want to preserve the order and add the call_site at the end?
> Trace events are not ABI, but if we don't have a strong reason to break it,
> I'd preserve the old order.

I checked recent trace patches and found that order changes is acceptable.

commit 8c39b8bc82aafcc8dd378bd79c76fac8e8a89c8d
Author: David Howells <dhowells@redhat.com>
Date:   Fri Jan 14 11:44:54 2022 +0000

    cachefiles: Make some tracepoint adjustments

-           TP_printk("o=%08x i=%lx e=%d",
-                     __entry->obj, __entry->ino, __entry->error)
+           TP_printk("o=%08x dB=%lx B=%lx e=%d",
+                     __entry->obj, __entry->dino, __entry->ino, __entry->error)

On the other hand I'm agree to keep old order by default.
that's why I added bytes_alloc and gfp_flags to end of output.
However I think call_site is an exception. In all cases found, 
call_site is output first.
For me personally it simplified output parsing.

So I would like to know Steven's position on this question.

Thank you,
	Vasily Averin

  reply	other threads:[~2022-05-11  5:11 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-06  4:46 [PATCH] percpu: improve percpu_alloc_percpu event trace Vasily Averin
2022-05-06  4:46 ` Vasily Averin
     [not found] ` <8d627f02-183f-c4e7-7c15-77b2b438536b-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2022-05-06  7:52   ` Vasily Averin
2022-05-06  7:52     ` Vasily Averin
     [not found]     ` <2b388d09-940e-990f-1f8a-2fdaa9210fa0-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2022-05-06 19:29       ` [PATCH v2] " Vasily Averin
2022-05-06 19:29         ` Vasily Averin
     [not found]         ` <a07be858-c8a3-7851-9086-e3262cbcf707-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2022-05-11  2:33           ` Roman Gushchin
2022-05-11  2:33             ` Roman Gushchin
2022-05-11  5:11             ` Vasily Averin [this message]
2022-05-11  5:11               ` Vasily Averin
     [not found]               ` <30a47b4e-7c4b-cd2d-998d-cfaf8d12d342-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2022-05-11 17:30                 ` Roman Gushchin
2022-05-11 17:30                   ` Roman Gushchin
2022-05-15 22:06             ` Steven Rostedt
2022-05-06 20:38   ` [PATCH] " kernel test robot
2022-05-06 20:38     ` kernel test robot
     [not found]     ` <202205070420.aAhuqpYk-lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2022-05-07 14:51       ` Vasily Averin
2022-05-07 14:51         ` Vasily Averin
2022-05-07 14:51         ` Vasily Averin
2022-05-07 19:02         ` [PATCH mm] tracing: incorrect gfp_t conversion Vasily Averin
2022-05-07 19:37           ` Andrew Morton
2022-05-07 22:28             ` Vasily Averin
2022-05-07 22:48               ` Andrew Morton
2022-05-07 23:00                 ` Andrew Morton
2022-05-08 20:37                   ` Matthew Wilcox
2022-05-08 20:51                 ` Joe Perches
2022-05-11  7:20                   ` [PATCH mm v2] " Vasily Averin
2022-05-15 22:09                     ` Steven Rostedt
2022-05-16 20:55                       ` Andrew Morton
     [not found]         ` <e1c09bbb-2c58-a986-c704-1db538da905a-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2022-05-09 21:06           ` [PATCH] percpu: improve percpu_alloc_percpu event trace Steven Rostedt
2022-05-09 21:06             ` Steven Rostedt
2022-05-09 21:06             ` Steven Rostedt
2022-05-10  4:22             ` Vasily Averin
2022-05-10  4:22               ` Vasily Averin
     [not found]               ` <6e68298c-7cdd-9984-215e-7e6fb3d03fe8-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2022-05-10 14:16                 ` Steven Rostedt
2022-05-10 14:16                   ` Steven Rostedt
2022-05-10 14:16                   ` Steven Rostedt

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=30a47b4e-7c4b-cd2d-998d-cfaf8d12d342@openvz.org \
    --to=vvs-gefaqzzx7r8dnm+yrofe0a@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org \
    --cc=dennis-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=kernel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \
    --cc=mhocko-IBi9RG/b67k@public.gmane.org \
    --cc=mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=roman.gushchin-fxUVXftIFDnyG1zEObXtfA@public.gmane.org \
    --cc=rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org \
    --cc=shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=vbabka-AlSwsSmVLrQ@public.gmane.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.