Re: [PATCH v2] percpu: improve percpu_alloc_percpu event trace

Vasily Averin
2022-05-11T05:12:04Z

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