tmpfs: Insert tmpfs cache pages to inactive list at first
in [Kernel]
|
Prev: Prevent reserving RAM in the region already reserved by BIOS
Next: linux-next: manual merge of the samsung tree with the tree
From: Wu Fengguang on 19 May 2010 21:10 Reviewed-by: Wu Fengguang <fengguang.wu(a)intel.com> The preceding comment "they need to go on the active_anon lru below" also needs update. Thanks, Fengguang -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Hugh Dickins on 20 May 2010 21:40 On Wed, 19 May 2010, KOSAKI Motohiro wrote: > Shaohua Li reported parallel file copy on tmpfs can lead to > OOM killer. This is regression of caused by commit 9ff473b9a7 > (vmscan: evict streaming IO first). Wow, It is 2 years old patch! > > Currently, tmpfs file cache is inserted active list at first. It > mean the insertion doesn't only increase numbers of pages in anon LRU, > but also reduce anon scanning ratio. Therefore, vmscan will get totally > confusion. It scan almost only file LRU even though the system have > plenty unused tmpfs pages. > > Historically, lru_cache_add_active_anon() was used by two reasons. > 1) Intend to priotize shmem page rather than regular file cache. > 2) Intend to avoid reclaim priority inversion of used once pages. > > But we've lost both motivation because (1) Now we have separate > anon and file LRU list. then, to insert active list doesn't help > such priotize. (2) In past, one pte access bit will cause page > activation. then to insert inactive list with pte access bit mean > higher priority than to insert active list. Its priority inversion > may lead to uninteded lru chun. but it was already solved by commit > 645747462 (vmscan: detect mapped file pages used only once). > (Thanks Hannes, you are great!) > > Thus, now we can use lru_cache_add_anon() instead. > > Reported-by: Shaohua Li <shaohua.li(a)intel.com> > Cc: Wu Fengguang <fengguang.wu(a)intel.com> > Cc: Johannes Weiner <hannes(a)cmpxchg.org> > Cc: Rik van Riel <riel(a)redhat.com> > Cc: Minchan Kim <minchan.kim(a)gmail.com> > Cc: Hugh Dickins <hughd(a)google.com> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> Acked-by: Hugh Dickins <hughd(a)google.com> Thanks - though I don't quite agree with your description: I can't see why the lru_cache_add_active_anon() was ever justified - that "active" came in along with the separate anon and file LRU lists. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Hugh Dickins on 20 May 2010 22:20 On Fri, 21 May 2010, KOSAKI Motohiro wrote: > > > Acked-by: Hugh Dickins <hughd(a)google.com> > > > > Thanks - though I don't quite agree with your description: I can't > > see why the lru_cache_add_active_anon() was ever justified - that > > "active" came in along with the separate anon and file LRU lists. > > If you have any worry, can you please share it? I'll test such workload > and fix the issue if necessary. You are expert than me in this area. ?? I've acked the patch: my worry is only with the detail of your comments on the history - in my view it was always wrong to put on the active LRU there, and I'm glad that you have now fixed it. If you really want to test some workload on 2.6.28 to see if it too works better with your fix, I won't stop you - but I'd much prefer you to be applying your mind to 2.6.35 and 2.6.36! Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Henrique de Moraes Holschuh on 21 May 2010 12:30 On Thu, 20 May 2010, Hugh Dickins wrote: > If you really want to test some workload on 2.6.28 to see if it too > works better with your fix, I won't stop you - but I'd much prefer > you to be applying your mind to 2.6.35 and 2.6.36! And sending it to -stable (after merged in mainline) if it should be on 2.6.32/33/34, please. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 21 May 2010 15:00
On Fri, 21 May 2010 09:36:50 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> wrote: > > Reviewed-by: Wu Fengguang <fengguang.wu(a)intel.com> > > > > The preceding comment "they need to go on the active_anon lru below" > > also needs update. > > > > Thanks. incremental patch is here. > > > --- > include/linux/swap.h | 10 ---------- > mm/filemap.c | 2 +- > 2 files changed, 1 insertions(+), 11 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 18420a9..4bfd932 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -224,21 +224,11 @@ static inline void lru_cache_add_anon(struct page *page) > __lru_cache_add(page, LRU_INACTIVE_ANON); > } > > -static inline void lru_cache_add_active_anon(struct page *page) > -{ > - __lru_cache_add(page, LRU_ACTIVE_ANON); > -} > - > static inline void lru_cache_add_file(struct page *page) > { > __lru_cache_add(page, LRU_INACTIVE_FILE); > } > > -static inline void lru_cache_add_active_file(struct page *page) > -{ > - __lru_cache_add(page, LRU_ACTIVE_FILE); > -} Did you intend to remove these two functions? They do appear to be unused now, but they still make sense and might be used in the future, perhaps. It's OK to remove them, but I'm wondering if it was deliberately included in this patch? > /* LRU Isolation modes. */ > #define ISOLATE_INACTIVE 0 /* Isolate inactive pages. */ > #define ISOLATE_ACTIVE 1 /* Isolate active pages. */ > diff --git a/mm/filemap.c b/mm/filemap.c > index 023ef61..a57931a 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -441,7 +441,7 @@ int add_to_page_cache_lru(struct page *page, struct address_space *mapping, > /* > * Splice_read and readahead add shmem/tmpfs pages into the page cache > * before shmem_readpage has a chance to mark them as SwapBacked: they > - * need to go on the active_anon lru below, and mem_cgroup_cache_charge > + * need to go on the anon lru below, and mem_cgroup_cache_charge > * (called in add_to_page_cache) needs to know where they're going too. > */ > if (mapping_cap_swap_backed(mapping)) > -- > 1.6.5.2 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |