Re: [PATCH 1/3] Add a new field to struct shrinker

[Mel Gorman <mgorman@techsingularity.net>]

On Fri, Jul 29, 2016 at 10:13:40AM +1000, Dave Chinner wrote:
> On Thu, Jul 28, 2016 at 11:25:13AM +0100, Mel Gorman wrote:
> > On Thu, Jul 28, 2016 at 03:49:47PM +1000, Dave Chinner wrote:
> > > Seems you're all missing the obvious.
> > > 
> > > Add a tracepoint for a shrinker callback that includes a "name"
> > > field, have the shrinker callback fill it out appropriately. e.g
> > > in the superblock shrinker:
> > > 
> > > 	trace_shrinker_callback(shrinker, shrink_control, sb->s_type->name);
> > > 
> > 
> > That misses capturing the latency of the call unless there is a begin/end
> > tracepoint.
> 
> Sure, but I didn't see that in the email talking about how to add a
> name. Even if it is a requirement, it's not necessary as we've
> already got shrinker runtime measurements from the
> trace_mm_shrink_slab_start and trace_mm_shrink_slab_end trace
> points. With the above callback event, shrinker call runtime is
> simply the time between the calls to the same shrinker within
> mm_shrink_slab start/end trace points.
> 

Fair point. It's not that hard to correlate them.

> <SNIP>
> 
> > My understanding was the point of the tracepoints was to get detailed
> > information on points where the kernel is known to stall for long periods
> > of time.
> 
> First I've heard that's what tracepoints are supposed to be used
> for.

I meant the specific case of trace_X_begin followed by trace_X_end, not
tracepoints in general.

-- 
Mel Gorman
SUSE Labs