All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: Dave Chinner <david@fromorbit.com>
Cc: Tony Jones <tonyj@suse.de>, Michal Hocko <mhocko@suse.cz>,
	Janani Ravichandran <janani.rvchndrn@gmail.com>,
	Rik van Riel <riel@surriel.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, hannes@cmpxchg.org,
	vdavydov@virtuozzo.com, vbabka@suse.cz,
	kirill.shutemov@linux.intel.com, bywxiaobai@163.com
Subject: Re: [PATCH 1/3] Add a new field to struct shrinker
Date: Fri, 29 Jul 2016 14:00:06 +0100	[thread overview]
Message-ID: <20160729130005.GE2799@techsingularity.net> (raw)
In-Reply-To: <20160729001340.GM12670@dastard>

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@techsingularity.net>
To: Dave Chinner <david@fromorbit.com>
Cc: Tony Jones <tonyj@suse.de>, Michal Hocko <mhocko@suse.cz>,
	Janani Ravichandran <janani.rvchndrn@gmail.com>,
	Rik van Riel <riel@surriel.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, hannes@cmpxchg.org,
	vdavydov@virtuozzo.com, vbabka@suse.cz,
	kirill.shutemov@linux.intel.com, bywxiaobai@163.com
Subject: Re: [PATCH 1/3] Add a new field to struct shrinker
Date: Fri, 29 Jul 2016 14:00:06 +0100	[thread overview]
Message-ID: <20160729130005.GE2799@techsingularity.net> (raw)
In-Reply-To: <20160729001340.GM12670@dastard>

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

  reply	other threads:[~2016-07-29 13:00 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-09  8:33 [PATCH 0/3] Add names of shrinkers and have tracepoints display them Janani Ravichandran
2016-07-09  8:33 ` Janani Ravichandran
2016-07-09  8:43 ` [PATCH 1/3] Add a new field to struct shrinker Janani Ravichandran
2016-07-09  8:43   ` Janani Ravichandran
2016-07-11  6:37   ` Michal Hocko
2016-07-11  6:37     ` Michal Hocko
2016-07-11 14:12     ` Rik van Riel
2016-07-11 14:33       ` Michal Hocko
2016-07-11 14:33         ` Michal Hocko
2016-07-20 14:41         ` Janani Ravichandran
2016-07-20 14:41           ` Janani Ravichandran
2016-07-20 14:54           ` Michal Hocko
2016-07-20 14:54             ` Michal Hocko
2016-07-23  1:27             ` Tony Jones
2016-07-23  1:27               ` Tony Jones
2016-07-23  4:05               ` Tony Jones
2016-07-23  4:05                 ` Tony Jones
2016-07-23 19:43                 ` Rik van Riel
2016-07-23 23:21                   ` Tony Jones
2016-07-23 23:21                     ` Tony Jones
2016-07-26 16:40             ` Tony Jones
2016-07-26 16:40               ` Tony Jones
2016-07-28  5:49               ` Dave Chinner
2016-07-28  5:49                 ` Dave Chinner
2016-07-28 10:25                 ` Mel Gorman
2016-07-28 10:25                   ` Mel Gorman
2016-07-29  0:13                   ` Dave Chinner
2016-07-29  0:13                     ` Dave Chinner
2016-07-29 13:00                     ` Mel Gorman [this message]
2016-07-29 13:00                       ` Mel Gorman
2016-08-12  3:09                       ` Tony Jones
2016-08-12  3:09                         ` Tony Jones
2016-07-09  8:52 ` [PATCH 2/3] Update name field for all shrinker instances Janani Ravichandran
2016-07-09  8:52   ` Janani Ravichandran
2016-07-13  0:43   ` Tony Jones
2016-07-13  0:43     ` Tony Jones
2016-07-09  9:05 ` [PATCH 3/3] Add name fields in shrinker tracepoint definitions Janani Ravichandran
2016-07-09  9:05   ` Janani Ravichandran
2016-07-09  9:45   ` kbuild test robot
2016-07-09 16:18     ` Janani Ravichandran
2016-07-11 14:18   ` Vlastimil Babka
2016-07-11 14:18     ` Vlastimil Babka
2016-07-13  0:35     ` Tony Jones
2016-07-13  0:35       ` Tony Jones
2016-07-13  6:16       ` Janani Ravichandran
2016-07-13  6:16         ` Janani Ravichandran
2016-07-13 19:12         ` Tony Jones
2016-07-13 19:12           ` Tony Jones
2016-07-13 19:48           ` Rik van Riel

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=20160729130005.GE2799@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=akpm@linux-foundation.org \
    --cc=bywxiaobai@163.com \
    --cc=david@fromorbit.com \
    --cc=hannes@cmpxchg.org \
    --cc=janani.rvchndrn@gmail.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=riel@surriel.com \
    --cc=tonyj@suse.de \
    --cc=vbabka@suse.cz \
    --cc=vdavydov@virtuozzo.com \
    /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.