[rh7,v2] mm/tcache: invalidate existing page during cleancache_put_page().

Submitted by Andrey Ryabinin on Jan. 18, 2018, 3:07 p.m.

Details

Message ID 20180118150702.14637-1-aryabinin@virtuozzo.com
State New
Series "mm/tcache: invalidate existing page during cleancache_put_page()."
Headers show

Commit Message

Andrey Ryabinin Jan. 18, 2018, 3:07 p.m.
We ->put_page() into tcache twice w/o ->get_page() in between, resulting in:

WARNING: CPU: 1 PID: 1936 at mm/tcache.c:752 tcache_attach_page+0x218/0x240

Call Trace:
 dump_stack+0x19/0x1b
 add_taint+0x32/0x70
 __warn+0xaa/0x100
 warn_slowpath_null+0x1d/0x20
 tcache_attach_page+0x218/0x240
 tcache_cleancache_put_page+0xdc/0x150
 __cleancache_put_page+0xa2/0xf0
 __delete_from_page_cache+0x309/0x370
 __remove_mapping+0x91/0x180
 shrink_page_list+0x5a8/0xa80
 shrink_inactive_list+0x1da/0x710
 shrink_lruvec+0x3a1/0x760
 shrink_zone+0x15b/0x310
 do_try_to_free_pages+0x1a0/0x610
 try_to_free_mem_cgroup_pages+0xeb/0x190
 mem_cgroup_reclaim+0x63/0x140
 try_charge+0x287/0x560
 mem_cgroup_try_charge+0x7a/0x130
 __add_to_page_cache_locked+0x97/0x2f0
 add_to_page_cache_lru+0x37/0xb0
 mpage_readpages+0xb5/0x150
 ext4_readpages+0x3c/0x40 [ext4]
 __do_page_cache_readahead+0x1cd/0x250
 ondemand_readahead+0x116/0x230
 page_cache_async_readahead+0xa8/0xc0

It's because we put all clean pages into cleancache, but in some cases
we may never read those pages from cleancache. E.g. see do_mpage_readpage(),
there are many case when cleancache_get_page() is not called.

Fix this by invalidating such existing page during ->put_page().

https://jira.sw.ru/browse/PSBM-80712
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---

Changes since v1:
 - add "ret = 0", as se must return number of put pages from tcache_cleancache_put_page()
 mm/tcache.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/mm/tcache.c b/mm/tcache.c
index a45af63fbd1b..9710531731ab 100644
--- a/mm/tcache.c
+++ b/mm/tcache.c
@@ -749,7 +749,6 @@  static int tcache_page_tree_insert(struct tcache_node *node, pgoff_t index,
 	}
 
 	err = radix_tree_insert(&node->page_tree, index, page);
-	WARN_ON(err == -EEXIST);
 	if (!err) {
 		if (!node->nr_pages++)
 			tcache_hold_node(node);
@@ -1256,9 +1255,18 @@  static int tcache_cleancache_put_page(int pool_id,
 		cache_page = tcache_alloc_page(node->pool);
 		if (cache_page) {
 			copy_highpage(cache_page, page);
-			if (tcache_attach_page(node, index, cache_page)) {
+			ret = tcache_attach_page(node, index, cache_page);
+			if (ret) {
+				if (ret == -EEXIST) {
+					struct page *page;
+
+					page = tcache_detach_page(node, index, false);
+					if (page)
+						tcache_put_page(page);
+				}
 				if (put_page_testzero(cache_page))
 					tcache_put_page(cache_page);
+				ret = 0;
 			} else
 				ret = 1;
 		}

Comments

Kirill Tkhai Jan. 18, 2018, 3:19 p.m.
On 18.01.2018 18:07, Andrey Ryabinin wrote:
> We ->put_page() into tcache twice w/o ->get_page() in between, resulting in:
> 
> WARNING: CPU: 1 PID: 1936 at mm/tcache.c:752 tcache_attach_page+0x218/0x240
> 
> Call Trace:
>  dump_stack+0x19/0x1b
>  add_taint+0x32/0x70
>  __warn+0xaa/0x100
>  warn_slowpath_null+0x1d/0x20
>  tcache_attach_page+0x218/0x240
>  tcache_cleancache_put_page+0xdc/0x150
>  __cleancache_put_page+0xa2/0xf0
>  __delete_from_page_cache+0x309/0x370
>  __remove_mapping+0x91/0x180
>  shrink_page_list+0x5a8/0xa80
>  shrink_inactive_list+0x1da/0x710
>  shrink_lruvec+0x3a1/0x760
>  shrink_zone+0x15b/0x310
>  do_try_to_free_pages+0x1a0/0x610
>  try_to_free_mem_cgroup_pages+0xeb/0x190
>  mem_cgroup_reclaim+0x63/0x140
>  try_charge+0x287/0x560
>  mem_cgroup_try_charge+0x7a/0x130
>  __add_to_page_cache_locked+0x97/0x2f0
>  add_to_page_cache_lru+0x37/0xb0
>  mpage_readpages+0xb5/0x150
>  ext4_readpages+0x3c/0x40 [ext4]
>  __do_page_cache_readahead+0x1cd/0x250
>  ondemand_readahead+0x116/0x230
>  page_cache_async_readahead+0xa8/0xc0
> 
> It's because we put all clean pages into cleancache, but in some cases
> we may never read those pages from cleancache. E.g. see do_mpage_readpage(),
> there are many case when cleancache_get_page() is not called.
> 
> Fix this by invalidating such existing page during ->put_page().
> 
> https://jira.sw.ru/browse/PSBM-80712
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>

> ---
> 
> Changes since v1:
>  - add "ret = 0", as se must return number of put pages from tcache_cleancache_put_page()
>  mm/tcache.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/tcache.c b/mm/tcache.c
> index a45af63fbd1b..9710531731ab 100644
> --- a/mm/tcache.c
> +++ b/mm/tcache.c
> @@ -749,7 +749,6 @@ static int tcache_page_tree_insert(struct tcache_node *node, pgoff_t index,
>  	}
>  
>  	err = radix_tree_insert(&node->page_tree, index, page);
> -	WARN_ON(err == -EEXIST);
>  	if (!err) {
>  		if (!node->nr_pages++)
>  			tcache_hold_node(node);
> @@ -1256,9 +1255,18 @@ static int tcache_cleancache_put_page(int pool_id,
>  		cache_page = tcache_alloc_page(node->pool);
>  		if (cache_page) {
>  			copy_highpage(cache_page, page);
> -			if (tcache_attach_page(node, index, cache_page)) {
> +			ret = tcache_attach_page(node, index, cache_page);
> +			if (ret) {
> +				if (ret == -EEXIST) {
> +					struct page *page;
> +
> +					page = tcache_detach_page(node, index, false);
> +					if (page)
> +						tcache_put_page(page);
> +				}
>  				if (put_page_testzero(cache_page))
>  					tcache_put_page(cache_page);
> +				ret = 0;
>  			} else
>  				ret = 1;
>  		}
>