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

Roman Gushchin
2022-05-11T17:30:52Z

On Wed, May 11, 2022 at 08:11:54AM +0300, Vasily Averin wrote:
> 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.

Ok, not a strong opinion, I think both options are acceptable.

Thanks!