Re: [PATCHv2 2/5] vmscan: Kill hibernation specific reclaim logic and unify it
From: Rafael J. Wysocki
Date: Sun Nov 01 2009 - 16:36:58 EST
On Sunday 01 November 2009, KOSAKI Motohiro wrote:
> shrink_all_zone() was introduced by commit d6277db4ab (swsusp: rework
> memory shrinker) for hibernate performance improvement. and sc.swap_cluster_max
> was introduced by commit a06fe4d307 (Speed freeing memory for suspend).
> commit a06fe4d307 said
> Without the patch:
> Freed 14600 pages in 1749 jiffies = 32.61 MB/s (Anomolous!)
> Freed 88563 pages in 14719 jiffies = 23.50 MB/s
> Freed 205734 pages in 32389 jiffies = 24.81 MB/s
> With the patch:
> Freed 68252 pages in 496 jiffies = 537.52 MB/s
> Freed 116464 pages in 569 jiffies = 798.54 MB/s
> Freed 209699 pages in 705 jiffies = 1161.89 MB/s
> At that time, their patch was pretty worth. However, Modern Hardware
> trend and recent VM improvement broke its worth. From several reason,
> I think we should remove shrink_all_zones() at all.
> 1) Old days, shrink_zone()'s slowness was mainly caused by stupid io-throttle
> at no i/o congestion.
> but current shrink_zone() is sane, not slow.
> 2) shrink_all_zone() try to shrink all pages at a time. but it doesn't works
> fine on numa system.
> System has 4GB memory and each node have 2GB. and hibernate need 1GB.
> steal 500MB from each node.
> steal 1GB from node-0.
> Oh, Cache balancing logic was broken. ;)
> Unfortunately, Desktop system moved ahead NUMA at nowadays.
> (Side note, if hibernate require 2GB, shrink_all_zones() never success
> on above machine)
> 3) if the node has several I/O flighting pages, shrink_all_zones() makes
> pretty bad result.
> schenario) hibernate need 1GB
> 1) shrink_all_zones() try to reclaim 1GB from Node-0
> 2) but it only reclaimed 990MB
> 3) stupidly, shrink_all_zones() try to reclaim 1GB from Node-1
> 4) it reclaimed 990MB
> Oh, well. it reclaimed twice much than required.
> In the other hand, current shrink_zone() has sane baling out logic.
> then, it doesn't make overkill reclaim. then, we lost shrink_zones()'s risk.
> 4) SplitLRU VM always keep active/inactive ratio very carefully. inactive list only
> shrinking break its assumption. it makes unnecessary OOM risk. it obviously suboptimal.
> Then, This patch changed shrink_all_memory() to only the wrapper function of
> do_try_to_free_pages(). it bring good reviewability and debuggability, and solve
> above problems.
> side note: Reclaim logic unificication makes two good side effect.
> - Fix recursive reclaim bug on shrink_all_memory().
> it did forgot to use PF_MEMALLOC. it mean the system be able to stuck into deadlock.
> - Now, shrink_all_memory() got lockdep awareness. it bring good debuggability.
As I said previously, I don't really see a reason to keep shrink_all_memory().
Do you think that removing it will result in performance degradation?
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/