All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com
Subject: Re: [PATCH 3/5] mm: vmscan: remove old flusher wakeup from direct reclaim path
Date: Thu, 26 Jan 2017 13:50:27 -0500	[thread overview]
Message-ID: <20170126185027.GB30636@cmpxchg.org> (raw)
In-Reply-To: <20170126100509.gbf6rxao6gsmqyq3@suse.de>

On Thu, Jan 26, 2017 at 10:05:09AM +0000, Mel Gorman wrote:
> On Mon, Jan 23, 2017 at 01:16:39PM -0500, Johannes Weiner wrote:
> > Direct reclaim has been replaced by kswapd reclaim in pretty much all
> > common memory pressure situations, so this code most likely doesn't
> > accomplish the described effect anymore. The previous patch wakes up
> > flushers for all reclaimers when we encounter dirty pages at the tail
> > end of the LRU. Remove the crufty old direct reclaim invocation.
> > 
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> In general I like this. I worried first that if kswapd is blocked
> writing pages that it won't reach the wakeup_flusher_threads but the
> previous patch handles it.
> 
> Now though, it occurs to me with the last patch that we always writeout
> the world when flushing threads. This may not be a great idea. Consider
> for example if there is a heavy writer of short-lived tmp files. In such a
> case, it is possible for the files to be truncated before they even hit the
> disk. However, if there are multiple "writeout the world" calls, these may
> now be hitting the disk. Furthermore, multiplle kswapd and direct reclaimers
> could all be requested to writeout the world and each request unplugs.
> 
> Is it possible to maintain the property of writing back pages relative
> to the numbers of pages scanned or have you determined already that it's
> not necessary?

That's what I started out with - waking the flushers for nr_taken. I
was using a silly test case that wrote < dirty background limit and
then allocated a burst of anon memory. When the dirty data is linear,
the bigger IO requests are beneficial. They don't exhaust struct
request (like kswapd 4k IO routinely does, and SWAP_CLUSTER_MAX is
only 32), and they require less frequent plugging.

Force-flushing temporary files under memory pressure is a concern -
although the most recently dirtied files would get queued last, giving
them still some time to get truncated - but I'm wary about splitting
the flush requests too aggressively when we DO sustain throngs of
dirty pages hitting the reclaim scanners.

I didn't test this with the real workload that gave us problems yet,
though, because deploying enough machines to get a good sample size
takes 1-2 days and to run through the full load spectrum another 4-5.
So it's harder to fine-tune these patches.

But this is a legit concern. I'll try to find out what happens when we
reduce the wakeups to nr_taken.

Given the problem these patches address, though, would you be okay
with keeping this patch in -mm? We're too far into 4.10 to merge it
upstream now, and I should have data on more precise wakeups before
the next merge window.

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: Johannes Weiner <hannes@cmpxchg.org>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com
Subject: Re: [PATCH 3/5] mm: vmscan: remove old flusher wakeup from direct reclaim path
Date: Thu, 26 Jan 2017 13:50:27 -0500	[thread overview]
Message-ID: <20170126185027.GB30636@cmpxchg.org> (raw)
In-Reply-To: <20170126100509.gbf6rxao6gsmqyq3@suse.de>

On Thu, Jan 26, 2017 at 10:05:09AM +0000, Mel Gorman wrote:
> On Mon, Jan 23, 2017 at 01:16:39PM -0500, Johannes Weiner wrote:
> > Direct reclaim has been replaced by kswapd reclaim in pretty much all
> > common memory pressure situations, so this code most likely doesn't
> > accomplish the described effect anymore. The previous patch wakes up
> > flushers for all reclaimers when we encounter dirty pages at the tail
> > end of the LRU. Remove the crufty old direct reclaim invocation.
> > 
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> In general I like this. I worried first that if kswapd is blocked
> writing pages that it won't reach the wakeup_flusher_threads but the
> previous patch handles it.
> 
> Now though, it occurs to me with the last patch that we always writeout
> the world when flushing threads. This may not be a great idea. Consider
> for example if there is a heavy writer of short-lived tmp files. In such a
> case, it is possible for the files to be truncated before they even hit the
> disk. However, if there are multiple "writeout the world" calls, these may
> now be hitting the disk. Furthermore, multiplle kswapd and direct reclaimers
> could all be requested to writeout the world and each request unplugs.
> 
> Is it possible to maintain the property of writing back pages relative
> to the numbers of pages scanned or have you determined already that it's
> not necessary?

That's what I started out with - waking the flushers for nr_taken. I
was using a silly test case that wrote < dirty background limit and
then allocated a burst of anon memory. When the dirty data is linear,
the bigger IO requests are beneficial. They don't exhaust struct
request (like kswapd 4k IO routinely does, and SWAP_CLUSTER_MAX is
only 32), and they require less frequent plugging.

Force-flushing temporary files under memory pressure is a concern -
although the most recently dirtied files would get queued last, giving
them still some time to get truncated - but I'm wary about splitting
the flush requests too aggressively when we DO sustain throngs of
dirty pages hitting the reclaim scanners.

I didn't test this with the real workload that gave us problems yet,
though, because deploying enough machines to get a good sample size
takes 1-2 days and to run through the full load spectrum another 4-5.
So it's harder to fine-tune these patches.

But this is a legit concern. I'll try to find out what happens when we
reduce the wakeups to nr_taken.

Given the problem these patches address, though, would you be okay
with keeping this patch in -mm? We're too far into 4.10 to merge it
upstream now, and I should have data on more precise wakeups before
the next merge window.

Thanks

  reply	other threads:[~2017-01-26 18:50 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-23 18:16 [PATCH 0/5] mm: vmscan: fix kswapd writeback regression Johannes Weiner
2017-01-23 18:16 ` Johannes Weiner
2017-01-23 18:16 ` [PATCH 1/5] mm: vmscan: scan dirty pages even in laptop mode Johannes Weiner
2017-01-23 18:16   ` Johannes Weiner
2017-01-26  1:27   ` Minchan Kim
2017-01-26  1:27     ` Minchan Kim
2017-01-26  9:52   ` Mel Gorman
2017-01-26  9:52     ` Mel Gorman
2017-01-26 13:13   ` Michal Hocko
2017-01-26 13:13     ` Michal Hocko
2017-01-23 18:16 ` [PATCH 2/5] mm: vmscan: kick flushers when we encounter dirty pages on the LRU Johannes Weiner
2017-01-23 18:16   ` Johannes Weiner
2017-01-26  1:35   ` Minchan Kim
2017-01-26  1:35     ` Minchan Kim
2017-01-26  9:57   ` Mel Gorman
2017-01-26  9:57     ` Mel Gorman
2017-01-26 17:47     ` Johannes Weiner
2017-01-26 17:47       ` Johannes Weiner
2017-01-26 18:47       ` Mel Gorman
2017-01-26 18:47         ` Mel Gorman
2017-01-26 13:16   ` Michal Hocko
2017-01-26 13:16     ` Michal Hocko
2017-01-23 18:16 ` [PATCH 3/5] mm: vmscan: remove old flusher wakeup from direct reclaim path Johannes Weiner
2017-01-23 18:16   ` Johannes Weiner
2017-01-26  1:38   ` Minchan Kim
2017-01-26  1:38     ` Minchan Kim
2017-01-26 10:05   ` Mel Gorman
2017-01-26 10:05     ` Mel Gorman
2017-01-26 18:50     ` Johannes Weiner [this message]
2017-01-26 18:50       ` Johannes Weiner
2017-01-26 20:45       ` Mel Gorman
2017-01-26 20:45         ` Mel Gorman
2017-01-27 12:01       ` Michal Hocko
2017-01-27 12:01         ` Michal Hocko
2017-01-27 14:27         ` Mel Gorman
2017-01-27 14:27           ` Mel Gorman
2017-01-26 13:21   ` Michal Hocko
2017-01-26 13:21     ` Michal Hocko
2017-01-23 18:16 ` [PATCH 4/5] mm: vmscan: only write dirty pages that the scanner has seen twice Johannes Weiner
2017-01-23 18:16   ` Johannes Weiner
2017-01-26  1:42   ` Minchan Kim
2017-01-26  1:42     ` Minchan Kim
2017-01-26 10:08   ` Mel Gorman
2017-01-26 10:08     ` Mel Gorman
2017-01-26 13:29   ` Michal Hocko
2017-01-26 13:29     ` Michal Hocko
2017-01-23 18:16 ` [PATCH 5/5] mm: vmscan: move dirty pages out of the way until they're flushed Johannes Weiner
2017-01-23 18:16   ` Johannes Weiner
2017-01-26  1:47   ` Minchan Kim
2017-01-26  1:47     ` Minchan Kim
2017-01-26 10:19   ` Mel Gorman
2017-01-26 10:19     ` Mel Gorman
2017-01-26 20:07     ` Johannes Weiner
2017-01-26 20:07       ` Johannes Weiner
2017-01-26 20:58       ` Mel Gorman
2017-01-26 20:58         ` Mel Gorman
2017-01-26 13:52   ` Michal Hocko
2017-01-26 13:52     ` Michal Hocko
2017-01-26  5:44 ` [PATCH 0/5] mm: vmscan: fix kswapd writeback regression Hillf Danton
2017-01-26  5:44   ` Hillf Danton

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=20170126185027.GB30636@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-team@fb.com \
    --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.