From: Andrea Arcangeli <aarcange@redhat.com> To: Mel Gorman <mel@csn.ul.ie> Cc: Johannes Weiner <hannes@cmpxchg.org>, Andrew Morton <akpm@linux-foundation.org>, Rik van Riel <riel@redhat.com>, Michal Hocko <mhocko@suse.cz>, Kent Overstreet <kent.overstreet@gmail.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [patch] vmscan: fix zone shrinking exit when scan work is done Date: Wed, 9 Feb 2011 19:28:46 +0100 [thread overview] Message-ID: <20110209182846.GN3347@random.random> (raw) In-Reply-To: <20110209164656.GA1063@csn.ul.ie> On Wed, Feb 09, 2011 at 04:46:56PM +0000, Mel Gorman wrote: > On Wed, Feb 09, 2011 at 04:46:06PM +0100, Johannes Weiner wrote: > > Hi, > > > > I think this should fix the problem of processes getting stuck in > > reclaim that has been reported several times. > > I don't think it's the only source but I'm basing this on seeing > constant looping in balance_pgdat() and calling congestion_wait() a few > weeks ago that I haven't rechecked since. However, this looks like a > real fix for a real problem. Agreed. Just yesterday I spent some time on the lumpy compaction changes after wondering about Michal's khugepaged 100% report, and I expected some fix was needed in this area (as I couldn't find any bug in khugepaged yet, so the lumpy compaction looked the next candidate for bugs). I've also been wondering about the !nr_scanned check in should_continue_reclaim too but I didn't look too much into the caller (I was tempted to remove it all together). I don't see how checking nr_scanned can be safe even after we fix the caller to avoid passing non-zero values if "goto restart". nr_scanned is incremented even for !page_evictable... so it's not really useful to insist, just because we scanned something, in my view. It looks bogus... So my proposal would be below. ==== Subject: mm: stop checking nr_scanned in should_continue_reclaim From: Andrea Arcangeli <aarcange@redhat.com> nr_scanned is incremented even for !page_evictable... so it's not really useful to insist, just because we scanned something. Signed-off-by: Andrea Arcangeli <aarcange@redhat.com> --- diff --git a/mm/vmscan.c b/mm/vmscan.c index 148c6e6..9741884 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1831,7 +1831,6 @@ out: */ static inline bool should_continue_reclaim(struct zone *zone, unsigned long nr_reclaimed, - unsigned long nr_scanned, struct scan_control *sc) { unsigned long pages_for_compaction; @@ -1841,15 +1840,8 @@ static inline bool should_continue_reclaim(struct zone *zone, if (!(sc->reclaim_mode & RECLAIM_MODE_COMPACTION)) return false; - /* - * If we failed to reclaim and have scanned the full list, stop. - * NOTE: Checking just nr_reclaimed would exit reclaim/compaction far - * faster but obviously would be less likely to succeed - * allocation. If this is desirable, use GFP_REPEAT to decide - * if both reclaimed and scanned should be checked or just - * reclaimed - */ - if (!nr_reclaimed && !nr_scanned) + /* If we failed to reclaim stop. */ + if (!nr_reclaimed) return false; /* @@ -1884,7 +1876,6 @@ static void shrink_zone(int priority, struct zone *zone, enum lru_list l; unsigned long nr_reclaimed; unsigned long nr_to_reclaim = sc->nr_to_reclaim; - unsigned long nr_scanned = sc->nr_scanned; restart: nr_reclaimed = 0; @@ -1923,8 +1914,7 @@ restart: shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0); /* reclaim/compaction might need reclaim to continue */ - if (should_continue_reclaim(zone, nr_reclaimed, - sc->nr_scanned - nr_scanned, sc)) + if (should_continue_reclaim(zone, nr_reclaimed, sc)) goto restart; throttle_vm_writeout(sc->gfp_mask);
WARNING: multiple messages have this Message-ID (diff)
From: Andrea Arcangeli <aarcange@redhat.com> To: Mel Gorman <mel@csn.ul.ie> Cc: Johannes Weiner <hannes@cmpxchg.org>, Andrew Morton <akpm@linux-foundation.org>, Rik van Riel <riel@redhat.com>, Michal Hocko <mhocko@suse.cz>, Kent Overstreet <kent.overstreet@gmail.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [patch] vmscan: fix zone shrinking exit when scan work is done Date: Wed, 9 Feb 2011 19:28:46 +0100 [thread overview] Message-ID: <20110209182846.GN3347@random.random> (raw) In-Reply-To: <20110209164656.GA1063@csn.ul.ie> On Wed, Feb 09, 2011 at 04:46:56PM +0000, Mel Gorman wrote: > On Wed, Feb 09, 2011 at 04:46:06PM +0100, Johannes Weiner wrote: > > Hi, > > > > I think this should fix the problem of processes getting stuck in > > reclaim that has been reported several times. > > I don't think it's the only source but I'm basing this on seeing > constant looping in balance_pgdat() and calling congestion_wait() a few > weeks ago that I haven't rechecked since. However, this looks like a > real fix for a real problem. Agreed. Just yesterday I spent some time on the lumpy compaction changes after wondering about Michal's khugepaged 100% report, and I expected some fix was needed in this area (as I couldn't find any bug in khugepaged yet, so the lumpy compaction looked the next candidate for bugs). I've also been wondering about the !nr_scanned check in should_continue_reclaim too but I didn't look too much into the caller (I was tempted to remove it all together). I don't see how checking nr_scanned can be safe even after we fix the caller to avoid passing non-zero values if "goto restart". nr_scanned is incremented even for !page_evictable... so it's not really useful to insist, just because we scanned something, in my view. It looks bogus... So my proposal would be below. ==== Subject: mm: stop checking nr_scanned in should_continue_reclaim From: Andrea Arcangeli <aarcange@redhat.com> nr_scanned is incremented even for !page_evictable... so it's not really useful to insist, just because we scanned something. Signed-off-by: Andrea Arcangeli <aarcange@redhat.com> --- diff --git a/mm/vmscan.c b/mm/vmscan.c index 148c6e6..9741884 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1831,7 +1831,6 @@ out: */ static inline bool should_continue_reclaim(struct zone *zone, unsigned long nr_reclaimed, - unsigned long nr_scanned, struct scan_control *sc) { unsigned long pages_for_compaction; @@ -1841,15 +1840,8 @@ static inline bool should_continue_reclaim(struct zone *zone, if (!(sc->reclaim_mode & RECLAIM_MODE_COMPACTION)) return false; - /* - * If we failed to reclaim and have scanned the full list, stop. - * NOTE: Checking just nr_reclaimed would exit reclaim/compaction far - * faster but obviously would be less likely to succeed - * allocation. If this is desirable, use GFP_REPEAT to decide - * if both reclaimed and scanned should be checked or just - * reclaimed - */ - if (!nr_reclaimed && !nr_scanned) + /* If we failed to reclaim stop. */ + if (!nr_reclaimed) return false; /* @@ -1884,7 +1876,6 @@ static void shrink_zone(int priority, struct zone *zone, enum lru_list l; unsigned long nr_reclaimed; unsigned long nr_to_reclaim = sc->nr_to_reclaim; - unsigned long nr_scanned = sc->nr_scanned; restart: nr_reclaimed = 0; @@ -1923,8 +1914,7 @@ restart: shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0); /* reclaim/compaction might need reclaim to continue */ - if (should_continue_reclaim(zone, nr_reclaimed, - sc->nr_scanned - nr_scanned, sc)) + if (should_continue_reclaim(zone, nr_reclaimed, sc)) goto restart; throttle_vm_writeout(sc->gfp_mask); -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-02-09 18:29 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-02-09 15:46 [patch] vmscan: fix zone shrinking exit when scan work is done Johannes Weiner 2011-02-09 15:46 ` Johannes Weiner 2011-02-09 15:54 ` Kent Overstreet 2011-02-09 15:54 ` Kent Overstreet 2011-02-09 16:46 ` Mel Gorman 2011-02-09 16:46 ` Mel Gorman 2011-02-09 18:28 ` Andrea Arcangeli [this message] 2011-02-09 18:28 ` Andrea Arcangeli 2011-02-09 20:05 ` Andrew Morton 2011-02-09 20:05 ` Andrew Morton 2011-02-10 10:21 ` Mel Gorman 2011-02-10 10:21 ` Mel Gorman 2011-02-10 10:41 ` Michal Hocko 2011-02-10 10:41 ` Michal Hocko 2011-02-10 12:48 ` Andrea Arcangeli 2011-02-10 12:48 ` Andrea Arcangeli 2011-02-10 13:33 ` Mel Gorman 2011-02-10 13:33 ` Mel Gorman 2011-02-10 14:14 ` Andrea Arcangeli 2011-02-10 14:14 ` Andrea Arcangeli 2011-02-10 14:58 ` Mel Gorman 2011-02-10 14:58 ` Mel Gorman 2011-02-16 9:50 ` [PATCH] mm: vmscan: Stop reclaim/compaction earlier due to insufficient progress if !__GFP_REPEAT Mel Gorman 2011-02-16 9:50 ` Mel Gorman 2011-02-16 10:13 ` Andrea Arcangeli 2011-02-16 10:13 ` Andrea Arcangeli 2011-02-16 11:22 ` Mel Gorman 2011-02-16 11:22 ` Mel Gorman 2011-02-16 14:44 ` Andrea Arcangeli 2011-02-16 14:44 ` Andrea Arcangeli 2011-02-16 12:03 ` Andrea Arcangeli 2011-02-16 12:03 ` Andrea Arcangeli 2011-02-16 12:14 ` Rik van Riel 2011-02-16 12:14 ` Rik van Riel 2011-02-16 12:38 ` Johannes Weiner 2011-02-16 12:38 ` Johannes Weiner 2011-02-16 23:26 ` Minchan Kim 2011-02-16 23:26 ` Minchan Kim 2011-02-17 22:22 ` Andrew Morton 2011-02-17 22:22 ` Andrew Morton 2011-02-18 12:22 ` Mel Gorman 2011-02-18 12:22 ` Mel Gorman 2011-02-10 4:04 ` [patch] vmscan: fix zone shrinking exit when scan work is done Minchan Kim 2011-02-10 4:04 ` 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=20110209182846.GN3347@random.random \ --to=aarcange@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=hannes@cmpxchg.org \ --cc=kent.overstreet@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mel@csn.ul.ie \ --cc=mhocko@suse.cz \ --cc=riel@redhat.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: linkBe 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.