From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f69.google.com (mail-wm0-f69.google.com [74.125.82.69]) by kanga.kvack.org (Postfix) with ESMTP id EE71F6B0260 for ; Mon, 24 Oct 2016 11:26:15 -0400 (EDT) Received: by mail-wm0-f69.google.com with SMTP id y138so34542639wme.7 for ; Mon, 24 Oct 2016 08:26:15 -0700 (PDT) Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com. [74.125.82.67]) by mx.google.com with ESMTPS id 5si12583639wmp.126.2016.10.24.08.26.14 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 08:26:14 -0700 (PDT) Received: by mail-wm0-f67.google.com with SMTP id f193so10317799wmg.3 for ; Mon, 24 Oct 2016 08:26:14 -0700 (PDT) From: Michal Hocko Subject: [Stable 4.4 - NEEDS REVIEW - 0/3] mm: working set fixes Date: Mon, 24 Oct 2016 17:26:02 +0200 Message-Id: <20161024152605.11707-1-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Antonio SJ Musumeci , Jan Kara , Linus Torvalds , Michal Hocko , Miklos Szeredi Hi Johannes, this is my attempt to backport your 22f2ac51b6d6 ("mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()") which has been marked for 3.15+ stable trees. There are 2 follow up fixes for this patch d3798ae8c6f3 ("mm: filemap: don't plant shadow entries without radix tree node") and 3ddf40e8c319 ("mm: filemap: fix mapping->nrpages double accounting in fuse") which are backported here as well. This is not an area I would feel really strongly so I would highly appreciate if you could review these backports. The first two needed quite some tweaking. Thanks! -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f72.google.com (mail-wm0-f72.google.com [74.125.82.72]) by kanga.kvack.org (Postfix) with ESMTP id 6C7A86B0261 for ; Mon, 24 Oct 2016 11:27:22 -0400 (EDT) Received: by mail-wm0-f72.google.com with SMTP id b80so34373900wme.5 for ; Mon, 24 Oct 2016 08:27:22 -0700 (PDT) Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com. [74.125.82.66]) by mx.google.com with ESMTPS id m63si12614763wma.64.2016.10.24.08.27.21 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 08:27:21 -0700 (PDT) Received: by mail-wm0-f66.google.com with SMTP id o81so10305403wma.2 for ; Mon, 24 Oct 2016 08:27:21 -0700 (PDT) From: Michal Hocko Subject: [Stable 4.4 - NEEDS REVIEW - 1/3] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Date: Mon, 24 Oct 2016 17:26:03 +0200 Message-Id: <20161024152605.11707-2-mhocko@kernel.org> In-Reply-To: <20161024152605.11707-1-mhocko@kernel.org> References: <20161024152605.11707-1-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Linus Torvalds , Michal Hocko , Antonio SJ Musumeci , Miklos Szeredi From: Johannes Weiner Commit 22f2ac51b6d643666f4db093f13144f773ff3f3a upstream. Antonio reports the following crash when using fuse under memory pressure: kernel BUG at /build/linux-a2WvEb/linux-4.4.0/mm/workingset.c:346! invalid opcode: 0000 [#1] SMP Modules linked in: all of them CPU: 2 PID: 63 Comm: kswapd0 Not tainted 4.4.0-36-generic #55-Ubuntu Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 04/27/2013 task: ffff88040cae6040 ti: ffff880407488000 task.ti: ffff880407488000 RIP: shadow_lru_isolate+0x181/0x190 Call Trace: __list_lru_walk_one.isra.3+0x8f/0x130 list_lru_walk_one+0x23/0x30 scan_shadow_nodes+0x34/0x50 shrink_slab.part.40+0x1ed/0x3d0 shrink_zone+0x2ca/0x2e0 kswapd+0x51e/0x990 kthread+0xd8/0xf0 ret_from_fork+0x3f/0x70 which corresponds to the following sanity check in the shadow node tracking: BUG_ON(node->count & RADIX_TREE_COUNT_MASK); The workingset code tracks radix tree nodes that exclusively contain shadow entries of evicted pages in them, and this (somewhat obscure) line checks whether there are real pages left that would interfere with reclaim of the radix tree node under memory pressure. While discussing ways how fuse might sneak pages into the radix tree past the workingset code, Miklos pointed to replace_page_cache_page(), and indeed there is a problem there: it properly accounts for the old page being removed - __delete_from_page_cache() does that - but then does a raw raw radix_tree_insert(), not accounting for the replacement page. Eventually the page count bits in node->count underflow while leaving the node incorrectly linked to the shadow node LRU. To address this, make sure replace_page_cache_page() uses the tracked page insertion code, page_cache_tree_insert(). This fixes the page accounting and makes sure page-containing nodes are properly unlinked from the shadow node LRU again. Also, make the sanity checks a bit less obscure by using the helpers for checking the number of pages and shadows in a radix tree node. Fixes: 449dd6984d0e ("mm: keep page cache radix tree nodes in check") Link: http://lkml.kernel.org/r/20160919155822.29498-1-hannes@cmpxchg.org Signed-off-by: Johannes Weiner Reported-by: Antonio SJ Musumeci Debugged-by: Miklos Szeredi Cc: [3.15+] Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Michal Hocko --- include/linux/swap.h | 2 ++ mm/filemap.c | 86 ++++++++++++++++++++++++++-------------------------- mm/workingset.c | 10 +++--- 3 files changed, 49 insertions(+), 49 deletions(-) diff --git a/include/linux/swap.h b/include/linux/swap.h index 7ba7dccaf0e7..b28de19aadbf 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -266,6 +266,7 @@ static inline void workingset_node_pages_inc(struct radix_tree_node *node) static inline void workingset_node_pages_dec(struct radix_tree_node *node) { + VM_BUG_ON(!workingset_node_pages(node)); node->count--; } @@ -281,6 +282,7 @@ static inline void workingset_node_shadows_inc(struct radix_tree_node *node) static inline void workingset_node_shadows_dec(struct radix_tree_node *node) { + VM_BUG_ON(!workingset_node_shadows(node)); node->count -= 1U << RADIX_TREE_COUNT_SHIFT; } diff --git a/mm/filemap.c b/mm/filemap.c index 1bb007624b53..4cfe423d3e8a 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -109,6 +109,48 @@ * ->tasklist_lock (memory_failure, collect_procs_ao) */ +static int page_cache_tree_insert(struct address_space *mapping, + struct page *page, void **shadowp) +{ + struct radix_tree_node *node; + void **slot; + int error; + + error = __radix_tree_create(&mapping->page_tree, page->index, + &node, &slot); + if (error) + return error; + if (*slot) { + void *p; + + p = radix_tree_deref_slot_protected(slot, &mapping->tree_lock); + if (!radix_tree_exceptional_entry(p)) + return -EEXIST; + if (shadowp) + *shadowp = p; + mapping->nrshadows--; + if (node) + workingset_node_shadows_dec(node); + } + radix_tree_replace_slot(slot, page); + mapping->nrpages++; + if (node) { + workingset_node_pages_inc(node); + /* + * Don't track node that contains actual pages. + * + * Avoid acquiring the list_lru lock if already + * untracked. The list_empty() test is safe as + * node->private_list is protected by + * mapping->tree_lock. + */ + if (!list_empty(&node->private_list)) + list_lru_del(&workingset_shadow_nodes, + &node->private_list); + } + return 0; +} + static void page_cache_tree_delete(struct address_space *mapping, struct page *page, void *shadow) { @@ -538,7 +580,7 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask) memcg = mem_cgroup_begin_page_stat(old); spin_lock_irqsave(&mapping->tree_lock, flags); __delete_from_page_cache(old, NULL, memcg); - error = radix_tree_insert(&mapping->page_tree, offset, new); + error = page_cache_tree_insert(mapping, new, NULL); BUG_ON(error); mapping->nrpages++; @@ -562,48 +604,6 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask) } EXPORT_SYMBOL_GPL(replace_page_cache_page); -static int page_cache_tree_insert(struct address_space *mapping, - struct page *page, void **shadowp) -{ - struct radix_tree_node *node; - void **slot; - int error; - - error = __radix_tree_create(&mapping->page_tree, page->index, - &node, &slot); - if (error) - return error; - if (*slot) { - void *p; - - p = radix_tree_deref_slot_protected(slot, &mapping->tree_lock); - if (!radix_tree_exceptional_entry(p)) - return -EEXIST; - if (shadowp) - *shadowp = p; - mapping->nrshadows--; - if (node) - workingset_node_shadows_dec(node); - } - radix_tree_replace_slot(slot, page); - mapping->nrpages++; - if (node) { - workingset_node_pages_inc(node); - /* - * Don't track node that contains actual pages. - * - * Avoid acquiring the list_lru lock if already - * untracked. The list_empty() test is safe as - * node->private_list is protected by - * mapping->tree_lock. - */ - if (!list_empty(&node->private_list)) - list_lru_del(&workingset_shadow_nodes, - &node->private_list); - } - return 0; -} - static int __add_to_page_cache_locked(struct page *page, struct address_space *mapping, pgoff_t offset, gfp_t gfp_mask, diff --git a/mm/workingset.c b/mm/workingset.c index aa017133744b..df66f426fdcf 100644 --- a/mm/workingset.c +++ b/mm/workingset.c @@ -341,21 +341,19 @@ static enum lru_status shadow_lru_isolate(struct list_head *item, * no pages, so we expect to be able to remove them all and * delete and free the empty node afterwards. */ - - BUG_ON(!node->count); - BUG_ON(node->count & RADIX_TREE_COUNT_MASK); + BUG_ON(!workingset_node_shadows(node)); + BUG_ON(workingset_node_pages(node)); for (i = 0; i < RADIX_TREE_MAP_SIZE; i++) { if (node->slots[i]) { BUG_ON(!radix_tree_exceptional_entry(node->slots[i])); node->slots[i] = NULL; - BUG_ON(node->count < (1U << RADIX_TREE_COUNT_SHIFT)); - node->count -= 1U << RADIX_TREE_COUNT_SHIFT; + workingset_node_shadows_dec(node); BUG_ON(!mapping->nrshadows); mapping->nrshadows--; } } - BUG_ON(node->count); + BUG_ON(workingset_node_shadows(node)); inc_zone_state(page_zone(virt_to_page(node)), WORKINGSET_NODERECLAIM); if (!__radix_tree_delete_node(&mapping->page_tree, node)) BUG(); -- 2.9.3 -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f72.google.com (mail-wm0-f72.google.com [74.125.82.72]) by kanga.kvack.org (Postfix) with ESMTP id 302976B0262 for ; Mon, 24 Oct 2016 11:27:24 -0400 (EDT) Received: by mail-wm0-f72.google.com with SMTP id d199so34751442wmd.0 for ; Mon, 24 Oct 2016 08:27:24 -0700 (PDT) Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com. [74.125.82.66]) by mx.google.com with ESMTPS id e133si12607718wma.73.2016.10.24.08.27.22 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 08:27:22 -0700 (PDT) Received: by mail-wm0-f66.google.com with SMTP id o81so10305510wma.2 for ; Mon, 24 Oct 2016 08:27:22 -0700 (PDT) From: Michal Hocko Subject: [Stable 4.4 - NEEDS REVIEW - 2/3] mm: filemap: don't plant shadow entries without radix tree node Date: Mon, 24 Oct 2016 17:26:04 +0200 Message-Id: <20161024152605.11707-3-mhocko@kernel.org> In-Reply-To: <20161024152605.11707-1-mhocko@kernel.org> References: <20161024152605.11707-1-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Linus Torvalds , Michal Hocko , Jan Kara From: Johannes Weiner Commit d3798ae8c6f3767c726403c2ca6ecc317752c9dd upstream. When the underflow checks were added to workingset_node_shadow_dec(), they triggered immediately: kernel BUG at ./include/linux/swap.h:276! invalid opcode: 0000 [#1] SMP Modules linked in: isofs usb_storage fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast ip6t_REJECT nf_reject_ipv6 soundcore wmi acpi_als pinctrl_sunrisepoint kfifo_buf tpm_tis industrialio acpi_pad pinctrl_intel tpm_tis_core tpm nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm_crypt CPU: 0 PID: 20929 Comm: blkid Not tainted 4.8.0-rc8-00087-gbe67d60ba944 #1 Hardware name: System manufacturer System Product Name/Z170-K, BIOS 1803 05/06/2016 task: ffff8faa93ecd940 task.stack: ffff8faa7f478000 RIP: page_cache_tree_insert+0xf1/0x100 Call Trace: __add_to_page_cache_locked+0x12e/0x270 add_to_page_cache_lru+0x4e/0xe0 mpage_readpages+0x112/0x1d0 blkdev_readpages+0x1d/0x20 __do_page_cache_readahead+0x1ad/0x290 force_page_cache_readahead+0xaa/0x100 page_cache_sync_readahead+0x3f/0x50 generic_file_read_iter+0x5af/0x740 blkdev_read_iter+0x35/0x40 __vfs_read+0xe1/0x130 vfs_read+0x96/0x130 SyS_read+0x55/0xc0 entry_SYSCALL_64_fastpath+0x13/0x8f Code: 03 00 48 8b 5d d8 65 48 33 1c 25 28 00 00 00 44 89 e8 75 19 48 83 c4 18 5b 41 5c 41 5d 41 5e 5d c3 0f 0b 41 bd ef ff ff ff eb d7 <0f> 0b e8 88 68 ef ff 0f 1f 84 00 RIP page_cache_tree_insert+0xf1/0x100 This is a long-standing bug in the way shadow entries are accounted in the radix tree nodes. The shrinker needs to know when radix tree nodes contain only shadow entries, no pages, so node->count is split in half to count shadows in the upper bits and pages in the lower bits. Unfortunately, the radix tree implementation doesn't know of this and assumes all entries are in node->count. When there is a shadow entry directly in root->rnode and the tree is later extended, the radix tree implementation will copy that entry into the new node and and bump its node->count, i.e. increases the page count bits. Once the shadow gets removed and we subtract from the upper counter, node->count underflows and triggers the warning. Afterwards, without node->count reaching 0 again, the radix tree node is leaked. Limit shadow entries to when we have actual radix tree nodes and can count them properly. That means we lose the ability to detect refaults from files that had only the first page faulted in at eviction time. Fixes: 449dd6984d0e ("mm: keep page cache radix tree nodes in check") Signed-off-by: Johannes Weiner Reported-and-tested-by: Linus Torvalds Reviewed-by: Jan Kara Cc: Andrew Morton Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds Signed-off-by: Michal Hocko --- include/linux/radix-tree.h | 3 +++ lib/radix-tree.c | 14 ++++++++++++++ mm/filemap.c | 48 +++++++++++++++++++++------------------------- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h index 5d5174b59802..1fe61d2a3875 100644 --- a/include/linux/radix-tree.h +++ b/include/linux/radix-tree.h @@ -271,6 +271,9 @@ bool __radix_tree_delete_node(struct radix_tree_root *root, struct radix_tree_node *node); void *radix_tree_delete_item(struct radix_tree_root *, unsigned long, void *); void *radix_tree_delete(struct radix_tree_root *, unsigned long); +void radix_tree_clear_tags(struct radix_tree_root *root, + struct radix_tree_node *node, + void **slot); unsigned int radix_tree_gang_lookup(struct radix_tree_root *root, void **results, unsigned long first_index, unsigned int max_items); diff --git a/lib/radix-tree.c b/lib/radix-tree.c index 6b79e9026e24..3ca8be1d1c70 100644 --- a/lib/radix-tree.c +++ b/lib/radix-tree.c @@ -1430,6 +1430,20 @@ void *radix_tree_delete(struct radix_tree_root *root, unsigned long index) } EXPORT_SYMBOL(radix_tree_delete); +void radix_tree_clear_tags(struct radix_tree_root *root, + struct radix_tree_node *node, + void **slot) +{ + if (node) { + unsigned int tag, offset = get_slot_offset(node, slot); + for (tag = 0; tag < RADIX_TREE_MAX_TAGS; tag++) + node_tag_clear(root, node, tag, offset); + } else { + /* Clear root node tags */ + root->gfp_mask &= __GFP_BITS_MASK; + } +} + /** * radix_tree_tagged - test whether any items in the tree are tagged * @root: radix tree root diff --git a/mm/filemap.c b/mm/filemap.c index 4cfe423d3e8a..c9c19c12f61d 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -155,44 +155,27 @@ static void page_cache_tree_delete(struct address_space *mapping, struct page *page, void *shadow) { struct radix_tree_node *node; - unsigned long index; - unsigned int offset; - unsigned int tag; void **slot; VM_BUG_ON(!PageLocked(page)); __radix_tree_lookup(&mapping->page_tree, page->index, &node, &slot); - if (shadow) { - mapping->nrshadows++; - /* - * Make sure the nrshadows update is committed before - * the nrpages update so that final truncate racing - * with reclaim does not see both counters 0 at the - * same time and miss a shadow entry. - */ - smp_wmb(); - } - mapping->nrpages--; + radix_tree_clear_tags(&mapping->page_tree, node, slot); if (!node) { - /* Clear direct pointer tags in root node */ - mapping->page_tree.gfp_mask &= __GFP_BITS_MASK; - radix_tree_replace_slot(slot, shadow); - return; - } - - /* Clear tree tags for the removed page */ - index = page->index; - offset = index & RADIX_TREE_MAP_MASK; - for (tag = 0; tag < RADIX_TREE_MAX_TAGS; tag++) { - if (test_bit(offset, node->tags[tag])) - radix_tree_tag_clear(&mapping->page_tree, index, tag); + /* + * We need a node to properly account shadow + * entries. Don't plant any without. XXX + */ + shadow = NULL; } /* Delete page, swap shadow entry */ radix_tree_replace_slot(slot, shadow); + if (!node) + goto out; + workingset_node_pages_dec(node); if (shadow) workingset_node_shadows_inc(node); @@ -212,6 +195,19 @@ static void page_cache_tree_delete(struct address_space *mapping, node->private_data = mapping; list_lru_add(&workingset_shadow_nodes, &node->private_list); } + +out: + if (shadow) { + mapping->nrshadows++; + /* + * Make sure the nrshadows update is committed before + * the nrpages update so that final truncate racing + * with reclaim does not see both counters 0 at the + * same time and miss a shadow entry. + */ + smp_wmb(); + } + mapping->nrpages--; } /* -- 2.9.3 -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f70.google.com (mail-wm0-f70.google.com [74.125.82.70]) by kanga.kvack.org (Postfix) with ESMTP id CD3566B0264 for ; Mon, 24 Oct 2016 11:27:25 -0400 (EDT) Received: by mail-wm0-f70.google.com with SMTP id y138so34560085wme.7 for ; Mon, 24 Oct 2016 08:27:25 -0700 (PDT) Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com. [74.125.82.65]) by mx.google.com with ESMTPS id q6si16637291wjr.172.2016.10.24.08.27.24 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 08:27:24 -0700 (PDT) Received: by mail-wm0-f65.google.com with SMTP id d199so10354727wmd.1 for ; Mon, 24 Oct 2016 08:27:24 -0700 (PDT) From: Michal Hocko Subject: [Stable 4.4 - NEEDS REVIEW - 3/3] mm: filemap: fix mapping->nrpages double accounting in fuse Date: Mon, 24 Oct 2016 17:26:05 +0200 Message-Id: <20161024152605.11707-4-mhocko@kernel.org> In-Reply-To: <20161024152605.11707-1-mhocko@kernel.org> References: <20161024152605.11707-1-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Miklos Szeredi , Linus Torvalds , Michal Hocko From: Johannes Weiner Commit 3ddf40e8c31964b744ff10abb48c8e36a83ec6e7 upstream. Commit 22f2ac51b6d6 ("mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()") switched replace_page_cache() from raw radix tree operations to page_cache_tree_insert() but didn't take into account that the latter function, unlike the raw radix tree op, handles mapping->nrpages. As a result, that counter is bumped for each page replacement rather than balanced out even. The mapping->nrpages counter is used to skip needless radix tree walks when invalidating, truncating, syncing inodes without pages, as well as statistics for userspace. Since the error is positive, we'll do more page cache tree walks than necessary; we won't miss a necessary one. And we'll report more buffer pages to userspace than there are. The error is limited to fuse inodes. Fixes: 22f2ac51b6d6 ("mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()") Signed-off-by: Johannes Weiner Cc: Andrew Morton Cc: Miklos Szeredi Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds Signed-off-by: Michal Hocko --- mm/filemap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mm/filemap.c b/mm/filemap.c index c9c19c12f61d..8675ce8a8f87 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -578,7 +578,6 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask) __delete_from_page_cache(old, NULL, memcg); error = page_cache_tree_insert(mapping, new, NULL); BUG_ON(error); - mapping->nrpages++; /* * hugetlb pages do not participate in page cache accounting. -- 2.9.3 -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f71.google.com (mail-wm0-f71.google.com [74.125.82.71]) by kanga.kvack.org (Postfix) with ESMTP id 97F646B0270 for ; Mon, 24 Oct 2016 14:52:35 -0400 (EDT) Received: by mail-wm0-f71.google.com with SMTP id y138so37528180wme.7 for ; Mon, 24 Oct 2016 11:52:35 -0700 (PDT) Received: from gum.cmpxchg.org (gum.cmpxchg.org. [85.214.110.215]) by mx.google.com with ESMTPS id b81si13649721wmc.136.2016.10.24.11.52.34 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 11:52:34 -0700 (PDT) Date: Mon, 24 Oct 2016 14:52:23 -0400 From: Johannes Weiner Subject: Re: [Stable 4.4 - NEEDS REVIEW - 1/3] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Message-ID: <20161024185223.GA28326@cmpxchg.org> References: <20161024152605.11707-1-mhocko@kernel.org> <20161024152605.11707-2-mhocko@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024152605.11707-2-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Michal Hocko Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Linus Torvalds , Michal Hocko , Antonio SJ Musumeci , Miklos Szeredi On Mon, Oct 24, 2016 at 05:26:03PM +0200, Michal Hocko wrote: > From: Johannes Weiner > > Commit 22f2ac51b6d643666f4db093f13144f773ff3f3a upstream. > > Antonio reports the following crash when using fuse under memory pressure: > > kernel BUG at /build/linux-a2WvEb/linux-4.4.0/mm/workingset.c:346! > invalid opcode: 0000 [#1] SMP > Modules linked in: all of them > CPU: 2 PID: 63 Comm: kswapd0 Not tainted 4.4.0-36-generic #55-Ubuntu > Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 04/27/2013 > task: ffff88040cae6040 ti: ffff880407488000 task.ti: ffff880407488000 > RIP: shadow_lru_isolate+0x181/0x190 > Call Trace: > __list_lru_walk_one.isra.3+0x8f/0x130 > list_lru_walk_one+0x23/0x30 > scan_shadow_nodes+0x34/0x50 > shrink_slab.part.40+0x1ed/0x3d0 > shrink_zone+0x2ca/0x2e0 > kswapd+0x51e/0x990 > kthread+0xd8/0xf0 > ret_from_fork+0x3f/0x70 > > which corresponds to the following sanity check in the shadow node > tracking: > > BUG_ON(node->count & RADIX_TREE_COUNT_MASK); > > The workingset code tracks radix tree nodes that exclusively contain > shadow entries of evicted pages in them, and this (somewhat obscure) > line checks whether there are real pages left that would interfere with > reclaim of the radix tree node under memory pressure. > > While discussing ways how fuse might sneak pages into the radix tree > past the workingset code, Miklos pointed to replace_page_cache_page(), > and indeed there is a problem there: it properly accounts for the old > page being removed - __delete_from_page_cache() does that - but then > does a raw raw radix_tree_insert(), not accounting for the replacement > page. Eventually the page count bits in node->count underflow while > leaving the node incorrectly linked to the shadow node LRU. > > To address this, make sure replace_page_cache_page() uses the tracked > page insertion code, page_cache_tree_insert(). This fixes the page > accounting and makes sure page-containing nodes are properly unlinked > from the shadow node LRU again. > > Also, make the sanity checks a bit less obscure by using the helpers for > checking the number of pages and shadows in a radix tree node. > > Fixes: 449dd6984d0e ("mm: keep page cache radix tree nodes in check") > Link: http://lkml.kernel.org/r/20160919155822.29498-1-hannes@cmpxchg.org > Signed-off-by: Johannes Weiner > Reported-by: Antonio SJ Musumeci > Debugged-by: Miklos Szeredi > Cc: [3.15+] > Signed-off-by: Andrew Morton > Signed-off-by: Linus Torvalds > Signed-off-by: Michal Hocko > --- > include/linux/swap.h | 2 ++ > mm/filemap.c | 86 ++++++++++++++++++++++++++-------------------------- > mm/workingset.c | 10 +++--- > 3 files changed, 49 insertions(+), 49 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 7ba7dccaf0e7..b28de19aadbf 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -266,6 +266,7 @@ static inline void workingset_node_pages_inc(struct radix_tree_node *node) > > static inline void workingset_node_pages_dec(struct radix_tree_node *node) > { > + VM_BUG_ON(!workingset_node_pages(node)); > node->count--; > } We should also pull 21f54ddae449 ("Using BUG_ON() as an assert() is _never_ acceptable") into stable on top of this patch to replace the BUG_ONs with warnings. Otherwise this looks good to me. -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f71.google.com (mail-wm0-f71.google.com [74.125.82.71]) by kanga.kvack.org (Postfix) with ESMTP id 8BA446B0272 for ; Mon, 24 Oct 2016 14:56:08 -0400 (EDT) Received: by mail-wm0-f71.google.com with SMTP id 79so37566199wmy.6 for ; Mon, 24 Oct 2016 11:56:08 -0700 (PDT) Received: from gum.cmpxchg.org (gum.cmpxchg.org. [85.214.110.215]) by mx.google.com with ESMTPS id r193si13741773wmf.14.2016.10.24.11.56.07 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 11:56:07 -0700 (PDT) Date: Mon, 24 Oct 2016 14:56:00 -0400 From: Johannes Weiner Subject: Re: [Stable 4.4 - NEEDS REVIEW - 2/3] mm: filemap: don't plant shadow entries without radix tree node Message-ID: <20161024185600.GB28326@cmpxchg.org> References: <20161024152605.11707-1-mhocko@kernel.org> <20161024152605.11707-3-mhocko@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024152605.11707-3-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Michal Hocko Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Linus Torvalds , Michal Hocko , Jan Kara Hi Michal, On Mon, Oct 24, 2016 at 05:26:04PM +0200, Michal Hocko wrote: > @@ -155,44 +155,27 @@ static void page_cache_tree_delete(struct address_space *mapping, > struct page *page, void *shadow) > { > struct radix_tree_node *node; > - unsigned long index; > - unsigned int offset; > - unsigned int tag; > void **slot; > > VM_BUG_ON(!PageLocked(page)); > > __radix_tree_lookup(&mapping->page_tree, page->index, &node, &slot); > > - if (shadow) { > - mapping->nrshadows++; > - /* > - * Make sure the nrshadows update is committed before > - * the nrpages update so that final truncate racing > - * with reclaim does not see both counters 0 at the > - * same time and miss a shadow entry. > - */ > - smp_wmb(); > - } > - mapping->nrpages--; > + radix_tree_clear_tags(&mapping->page_tree, node, slot); > > if (!node) { > - /* Clear direct pointer tags in root node */ > - mapping->page_tree.gfp_mask &= __GFP_BITS_MASK; > - radix_tree_replace_slot(slot, shadow); > - return; > - } There is no need to include the refactoring of the tag clearing in the stable backport. I already sent a simpler backport of this patch for 4.4 to Greg, attached here for reference: From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f70.google.com (mail-wm0-f70.google.com [74.125.82.70]) by kanga.kvack.org (Postfix) with ESMTP id D185D6B0274 for ; Mon, 24 Oct 2016 14:56:57 -0400 (EDT) Received: by mail-wm0-f70.google.com with SMTP id c78so36970116wme.4 for ; Mon, 24 Oct 2016 11:56:57 -0700 (PDT) Received: from gum.cmpxchg.org (gum.cmpxchg.org. [85.214.110.215]) by mx.google.com with ESMTPS id 186si13706597wmz.89.2016.10.24.11.56.56 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 11:56:56 -0700 (PDT) Date: Mon, 24 Oct 2016 14:56:50 -0400 From: Johannes Weiner Subject: Re: [Stable 4.4 - NEEDS REVIEW - 3/3] mm: filemap: fix mapping->nrpages double accounting in fuse Message-ID: <20161024185650.GC28326@cmpxchg.org> References: <20161024152605.11707-1-mhocko@kernel.org> <20161024152605.11707-4-mhocko@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024152605.11707-4-mhocko@kernel.org> Sender: owner-linux-mm@kvack.org List-ID: To: Michal Hocko Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Miklos Szeredi , Linus Torvalds , Michal Hocko This one looks good to me. Thanks -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f70.google.com (mail-wm0-f70.google.com [74.125.82.70]) by kanga.kvack.org (Postfix) with ESMTP id 8A6F2280250 for ; Mon, 24 Oct 2016 14:58:51 -0400 (EDT) Received: by mail-wm0-f70.google.com with SMTP id b80so37385161wme.5 for ; Mon, 24 Oct 2016 11:58:51 -0700 (PDT) Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com. [74.125.82.68]) by mx.google.com with ESMTPS id b10si13737778wmg.36.2016.10.24.11.58.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 11:58:50 -0700 (PDT) Received: by mail-wm0-f68.google.com with SMTP id o81so11139405wma.2 for ; Mon, 24 Oct 2016 11:58:50 -0700 (PDT) Date: Mon, 24 Oct 2016 20:58:48 +0200 From: Michal Hocko Subject: Re: [Stable 4.4 - NEEDS REVIEW - 1/3] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Message-ID: <20161024185848.GD13148@dhcp22.suse.cz> References: <20161024152605.11707-1-mhocko@kernel.org> <20161024152605.11707-2-mhocko@kernel.org> <20161024185223.GA28326@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024185223.GA28326@cmpxchg.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Linus Torvalds , Antonio SJ Musumeci , Miklos Szeredi On Mon 24-10-16 14:52:23, Johannes Weiner wrote: > On Mon, Oct 24, 2016 at 05:26:03PM +0200, Michal Hocko wrote: [...] > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > index 7ba7dccaf0e7..b28de19aadbf 100644 > > --- a/include/linux/swap.h > > +++ b/include/linux/swap.h > > @@ -266,6 +266,7 @@ static inline void workingset_node_pages_inc(struct radix_tree_node *node) > > > > static inline void workingset_node_pages_dec(struct radix_tree_node *node) > > { > > + VM_BUG_ON(!workingset_node_pages(node)); > > node->count--; > > } > > We should also pull 21f54ddae449 ("Using BUG_ON() as an assert() is > _never_ acceptable") into stable on top of this patch to replace the > BUG_ONs with warnings. > > Otherwise this looks good to me. OK, I have put that one on top and after the review will post it in one series. Thanks for the review Johannes! -- Michal Hocko SUSE Labs -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f71.google.com (mail-wm0-f71.google.com [74.125.82.71]) by kanga.kvack.org (Postfix) with ESMTP id F39B16B0253 for ; Mon, 24 Oct 2016 15:08:19 -0400 (EDT) Received: by mail-wm0-f71.google.com with SMTP id b80so37531769wme.5 for ; Mon, 24 Oct 2016 12:08:19 -0700 (PDT) Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com. [74.125.82.68]) by mx.google.com with ESMTPS id s76si13784826wmb.46.2016.10.24.12.08.18 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 12:08:18 -0700 (PDT) Received: by mail-wm0-f68.google.com with SMTP id d199so11220351wmd.1 for ; Mon, 24 Oct 2016 12:08:18 -0700 (PDT) Date: Mon, 24 Oct 2016 21:08:17 +0200 From: Michal Hocko Subject: Re: [Stable 4.4 - NEEDS REVIEW - 2/3] mm: filemap: don't plant shadow entries without radix tree node Message-ID: <20161024190816.GE13148@dhcp22.suse.cz> References: <20161024152605.11707-1-mhocko@kernel.org> <20161024152605.11707-3-mhocko@kernel.org> <20161024185600.GB28326@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024185600.GB28326@cmpxchg.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Stable tree , linux-mm@kvack.org, LKML , Andrew Morton , Linus Torvalds , Jan Kara On Mon 24-10-16 14:56:00, Johannes Weiner wrote: > Hi Michal, > > On Mon, Oct 24, 2016 at 05:26:04PM +0200, Michal Hocko wrote: > > @@ -155,44 +155,27 @@ static void page_cache_tree_delete(struct address_space *mapping, > > struct page *page, void *shadow) > > { > > struct radix_tree_node *node; > > - unsigned long index; > > - unsigned int offset; > > - unsigned int tag; > > void **slot; > > > > VM_BUG_ON(!PageLocked(page)); > > > > __radix_tree_lookup(&mapping->page_tree, page->index, &node, &slot); > > > > - if (shadow) { > > - mapping->nrshadows++; > > - /* > > - * Make sure the nrshadows update is committed before > > - * the nrpages update so that final truncate racing > > - * with reclaim does not see both counters 0 at the > > - * same time and miss a shadow entry. > > - */ > > - smp_wmb(); > > - } > > - mapping->nrpages--; > > + radix_tree_clear_tags(&mapping->page_tree, node, slot); > > > > if (!node) { > > - /* Clear direct pointer tags in root node */ > > - mapping->page_tree.gfp_mask &= __GFP_BITS_MASK; > > - radix_tree_replace_slot(slot, shadow); > > - return; > > - } > > There is no need to include the refactoring of the tag clearing in the > stable backport. I already sent a simpler backport of this patch for > 4.4 to Greg, attached here for reference: I do not see this in 4.4 so maybe it's fallen through cracks. Yours definitely looks easier and I will use it. I will post all 4 patches for inclusion for stable tomorrow unless something else pops out. Thanks for the review again! -- Michal Hocko SUSE Labs -- 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: email@kvack.org