All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] mm: fix pgalloc_stall on unpopulated zone
Date: Thu, 14 Jul 2016 17:58:27 +0900	[thread overview]
Message-ID: <20160714085827.GA9480@bbox> (raw)
In-Reply-To: <20160714085153.GL11400@suse.de>

On Thu, Jul 14, 2016 at 09:51:53AM +0100, Mel Gorman wrote:
> On Thu, Jul 14, 2016 at 10:11:19AM +0900, Minchan Kim wrote:
> > > The patch means that the vmstat accounting and tracepoint data is also
> > > out of sync. One thing I wanted to be able to do was
> > > 
> > > 1. Observe that there are alloc stalls on DMA32 or some other low zone
> > > 2. Activate mm_vmscan_direct_reclaim_begin, filter on classzone_idx ==
> > >    DMA32 and identify the source of the lowmem allocations
> > > 
> > > If your patch is applied, I cannot depend on the stall stats any more
> > > and the tracepoint is required to determine if there really any
> > > zone-contrained allocations. It can be *inferred* from the skip stats
> > > but only if such skips occurred and that is not guaranteed.
> > 
> > Just a nit:
> > 
> > Hmm, can't we omit classzone_idx in mm_vm_scan_direct_begin_template?
> > Because every functions already have gfp_flags so that we can classzone_idx
> > via gfp_zone(gfp_flags) without passing it.
> > 
> 
> We could but it's potentially wrong. classzone_idx *should* be derived
> from the gfp_flags but it's possible a bug would lead it to be another
> value. The saving from passing it in is marginal at best.
> 
> If it's omitted from the tracepoint itself, there is a small amount of
> disk saving which is potentially significant if there is a lot of direct
> reclaim. Unfortunately, it also makes it harder to filter that
> tracepoint because the filter rules must be an implementation of
> gfp_zone.
> 
> Right now I believe the saving is marginal and the cost of potentially
> using the wrong information or making the filtering harder offsets that
> marginal saving.

Agreed.

Thanks.

--
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: Minchan Kim <minchan@kernel.org>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	<linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>
Subject: Re: [PATCH] mm: fix pgalloc_stall on unpopulated zone
Date: Thu, 14 Jul 2016 17:58:27 +0900	[thread overview]
Message-ID: <20160714085827.GA9480@bbox> (raw)
In-Reply-To: <20160714085153.GL11400@suse.de>

On Thu, Jul 14, 2016 at 09:51:53AM +0100, Mel Gorman wrote:
> On Thu, Jul 14, 2016 at 10:11:19AM +0900, Minchan Kim wrote:
> > > The patch means that the vmstat accounting and tracepoint data is also
> > > out of sync. One thing I wanted to be able to do was
> > > 
> > > 1. Observe that there are alloc stalls on DMA32 or some other low zone
> > > 2. Activate mm_vmscan_direct_reclaim_begin, filter on classzone_idx ==
> > >    DMA32 and identify the source of the lowmem allocations
> > > 
> > > If your patch is applied, I cannot depend on the stall stats any more
> > > and the tracepoint is required to determine if there really any
> > > zone-contrained allocations. It can be *inferred* from the skip stats
> > > but only if such skips occurred and that is not guaranteed.
> > 
> > Just a nit:
> > 
> > Hmm, can't we omit classzone_idx in mm_vm_scan_direct_begin_template?
> > Because every functions already have gfp_flags so that we can classzone_idx
> > via gfp_zone(gfp_flags) without passing it.
> > 
> 
> We could but it's potentially wrong. classzone_idx *should* be derived
> from the gfp_flags but it's possible a bug would lead it to be another
> value. The saving from passing it in is marginal at best.
> 
> If it's omitted from the tracepoint itself, there is a small amount of
> disk saving which is potentially significant if there is a lot of direct
> reclaim. Unfortunately, it also makes it harder to filter that
> tracepoint because the filter rules must be an implementation of
> gfp_zone.
> 
> Right now I believe the saving is marginal and the cost of potentially
> using the wrong information or making the filtering harder offsets that
> marginal saving.

Agreed.

Thanks.

  reply	other threads:[~2016-07-14  8:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-13  2:24 [PATCH] mm: fix pgalloc_stall on unpopulated zone Minchan Kim
2016-07-13  2:24 ` Minchan Kim
2016-07-13  9:25 ` Mel Gorman
2016-07-13  9:25   ` Mel Gorman
2016-07-14  1:11   ` Minchan Kim
2016-07-14  1:11     ` Minchan Kim
2016-07-14  8:51     ` Mel Gorman
2016-07-14  8:51       ` Mel Gorman
2016-07-14  8:58       ` Minchan Kim [this message]
2016-07-14  8:58         ` Minchan Kim

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=20160714085827.GA9480@bbox \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    /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.