On Mon, Dec 02, 2019 at 12:14:15PM -0500, Steven Rostedt wrote:
> On Sat, 30 Nov 2019 17:50:30 -0800
> akpm@xxxxxxxxxxxxxxxxxxxx wrote:
>
> > /* Optimized variant when page is already known not to be PageAnon */
> > --- a/include/trace/events/kmem.h~mm-emit-tracepoint-when-rss-changes
> > +++ a/include/trace/events/kmem.h
> > @@ -316,6 +316,27 @@ TRACE_EVENT(mm_page_alloc_extfrag,
> > __entry->change_ownership)
> > );
> >
> > +TRACE_EVENT(rss_stat,
> > +
> > + TP_PROTO(int member,
> > + long count),
> > +
> > + TP_ARGS(member, count),
> > +
> > + TP_STRUCT__entry(
> > + __field(int, member)
> > + __field(long, size)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->member = member;
> > + __entry->size = (count << PAGE_SHIFT);
>
> It's best to put all calculations (including shifts) in the print part,
> as that's the slow path. The TP_fast_assign() is done when the trace
> point is triggered (during the execution of the code). It's best to
> keep this in the slow path (TP_printk).
>
> __entry->count = count;
>
> > + ),
> > +
> > + TP_printk("member=%d size=%ldB",
> > + __entry->member,
> > + __entry->size)
>
> __entry->count << PAGE_SHIFT)
Ah. Android users now use a tool called perfetto which gather raw trace
(binary format). So such shifting will have to be done by userspace then if
we did it this way. And I'm afraid this patch has been in circulation for
quite some time now that may cause major pains in changing userspace tooling
now :-\
I would say lets leave it alone for this once! But that is a good idea.
thanks,
- Joel
