Re: page eviction from the buddy cache

[Alexey Lyahkov <alexey.lyashkov@gmail.com>]

Mel,


On Apr 25, 2013, at 17:30, Mel Gorman wrote:

> On Wed, Apr 24, 2013 at 10:26:50AM -0400, Theodore Ts'o wrote:
>> On Tue, Apr 23, 2013 at 03:00:08PM -0700, Andrew Morton wrote:
>>> That should fix things for now.  Although it might be better to just do
>>> 
>>> 	mark_page_accessed(page);	/* to SetPageReferenced */
>>> 	lru_add_drain();		/* to SetPageLRU */
>>> 
>>> Because a) this was too early to decide that the page is
>>> super-important and b) the second touch of this page should have a
>>> mark_page_accessed() in it already.
>> 
>> The question is do we really want to put lru_add_drain() into the ext4
>> file system code?  That seems to pushing some fairly mm-specific
>> knowledge into file system code.  I'll do this if I have to do, but
>> wouldn't be better if this was pushed into mark_page_accessed(), or
>> some other new API was exported by the mm subsystem?
>> 
> 
> I don't think we want to push lru_add_drain() into the ext4 code. It's
> too specific of knowledge just to work around pagevecs. Before we rework
> how pagevecs select what LRU to place a page, can we make sure that fixing
> that will fix the problem?
> 
what is "that"? puting lru_add_drain() in ext4 core? sure that is fixes problem with many small reads during large write.
originally i have put shake_page() in ext4 code, but that have call lru_add_drain_all() so to exaggerated.

Index: linux-stage/fs/ext4/mballoc.c
===================================================================
--- linux-stage.orig/fs/ext4/mballoc.c	2013-03-19 10:55:52.000000000 +0200
+++ linux-stage/fs/ext4/mballoc.c	2013-03-19 10:59:02.000000000 +0200
@@ -900,8 +900,11 @@ static int ext4_mb_init_cache(struct pag
 			incore = data;
 		}
 	}
-	if (likely(err == 0))
+	if (likely(err == 0)) {
 		SetPageUptodate(page);
+		/* make sure it's in active list */
+		mark_page_accessed(page);
+	}
 
 out:
 	if (bh) {
@@ -957,6 +960,8 @@ int ext4_mb_init_group(struct super_bloc
 	page = find_or_create_page(inode->i_mapping, pnum, GFP_NOFS);
 	if (page) {
 		BUG_ON(page->mapping != inode->i_mapping);
+		/* move to lru - should be lru_add_drain() */
+		shake_page(page, 0);
 		ret = ext4_mb_init_cache(page, NULL);
 		if (ret) {
 			unlock_page(page);
@@ -986,6 +991,8 @@ int ext4_mb_init_group(struct super_bloc
 		unlock_page(page);
 	} else if (page) {
 		BUG_ON(page->mapping != inode->i_mapping);
+		/* move to lru - should be lru_add_drain() */
+		shake_page(page, 0);
 		ret = ext4_mb_init_cache(page, bitmap);
 		if (ret) {
 			unlock_page(page);
@@ -1087,6 +1094,7 @@ repeat_load_buddy:
 		if (page) {
 			BUG_ON(page->mapping != inode->i_mapping);
 			if (!PageUptodate(page)) {
+				shake_page(page, 0);
 				ret = ext4_mb_init_cache(page, NULL);
 				if (ret) {
 					unlock_page(page);
@@ -1118,6 +1126,7 @@ repeat_load_buddy:
 		if (page) {
 			BUG_ON(page->mapping != inode->i_mapping);
 			if (!PageUptodate(page)) {
+				shake_page(page, 0);
 				ret = ext4_mb_init_cache(page, e4b->bd_bitmap);
 				if (ret) {
 					unlock_page(page);
@@ -2500,6 +2509,8 @@ static int ext4_mb_init_backend(struct s
 	 * not in the inode hash, so it should never be found by iget(), but
 	 * this will avoid confusion if it ever shows up during debugging. */
 	sbi->s_buddy_cache->i_ino = EXT4_BAD_INO;
+	sbi->s_buddy_cache->i_state = I_NEW;
+//	mapping_set_unevictable(sbi->s_buddy_cache->i_mapping);
 	EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
 	for (i = 0; i < ngroups; i++) {
 		desc = ext4_get_group_desc(sb, i, NULL);


additional i_state = I_NEW need to prevent kill page cache from sysctl -w vm.drop_caches=3

> Andrew, can you try the following patch please? Also, is there any chance
> you can describe in more detail what the workload does?
lustre OSS node + IOR with file size twice more then OSS memory.

> If it fails to boot,
> remove the second that calls lru_add_drain_all() and try again.
well, i will try.

> 
> The patch looks deceptively simple, a downside from is is that workloads that
> call mark_page_accessed() frequently will contend more on the zone->lru_lock
> than it did previously. Moving lru_add_drain() to the ext4 could would
> suffer the same contention problem.
NO, isn't. we have call lru_add_drain() in new page allocation case, but mark_page_accessed called without differences - is page in page cache already or it's new allocated - so we have very small zone->lru_lock contention.


> 
> Thanks.
> 
> ---8<---
> mm: pagevec: Move inactive pages to active lists even if on a pagevec
> 
> If a page is on a pagevec aimed at the inactive list then two subsequent
> calls to mark_page_acessed() will still not move it to the active list.
> This can cause a page to be reclaimed sooner than is expected. This
> patch detects if an inactive page is not on the LRU and drains the
> pagevec before promoting it.
> 
> Not-signed-off
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 8a529a0..eac64fe 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -437,7 +437,18 @@ void activate_page(struct page *page)
> void mark_page_accessed(struct page *page)
> {
> 	if (!PageActive(page) && !PageUnevictable(page) &&
> -			PageReferenced(page) && PageLRU(page)) {
> +			PageReferenced(page)) {
> +		/* Page could be in pagevec */
> +		if (!PageLRU(page))
> +			lru_add_drain();
> +
> +		/*
> +		 * Weeeee, using in_atomic() like this is a hand-grenade.
> +		 * Patch is for debugging purposes only, do not merge this.
> +		 */
> +		if (!PageLRU(page) && !in_atomic())
> +			lru_add_drain_all();
> +
> 		activate_page(page);
> 		ClearPageReferenced(page);
> 	} else if (!PageReferenced(page)) {

--
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>