linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH stable 4.4 0/4] mm: workingset backports
@ 2016-10-25  7:51 Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 1/4] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Michal Hocko
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Michal Hocko @ 2016-10-25  7:51 UTC (permalink / raw)
  To: Stable tree
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton,
	Antonio SJ Musumeci, Jan Kara, Linus Torvalds, Michal Hocko,
	Miklos Szeredi

Hi,
here is the backport of (hopefully) all workingset related fixes for
4.4 kernel. The series has been reviewed by Johannes [1]. The main
motivation for the backport is 22f2ac51b6d6 ("mm: workingset: fix crash
in shadow node shrinker caused by replace_page_cache_page()") which is
a fix for a triggered BUG_ON. This is not sufficient because there are
follow up fixes which were introduced later.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH stable 4.4 1/4] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
@ 2016-10-25  7:51 ` Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node Michal Hocko
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Michal Hocko @ 2016-10-25  7:51 UTC (permalink / raw)
  To: Stable tree
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton, Linus Torvalds,
	Michal Hocko, Antonio SJ Musumeci, Miklos Szeredi

From: Johannes Weiner <hannes@cmpxchg.org>

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.

[mhocko@suse.com: backport for 4.4]
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 <hannes@cmpxchg.org>
Reported-by: Antonio SJ Musumeci <trapexit@spawn.link>
Debugged-by: Miklos Szeredi <miklos@szeredi.hu>
Cc: <stable@vger.kernel.org>	[3.15+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 1/4] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Michal Hocko
@ 2016-10-25  7:51 ` Michal Hocko
  2016-10-26 12:45   ` Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 3/4] mm: filemap: fix mapping->nrpages double accounting in fuse Michal Hocko
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Michal Hocko @ 2016-10-25  7:51 UTC (permalink / raw)
  To: Stable tree
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton, Linus Torvalds,
	Michal Hocko, Jan Kara

From: Johannes Weiner <hannes@cmpxchg.org>

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.

[hannes@cmpxchg.org: backport for 4.4 stable]
Fixes: 449dd6984d0e ("mm: keep page cache radix tree nodes in check")
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/filemap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/mm/filemap.c b/mm/filemap.c
index 4cfe423d3e8a..7ad648c9780c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -164,6 +164,14 @@ static void page_cache_tree_delete(struct address_space *mapping,
 
 	__radix_tree_lookup(&mapping->page_tree, page->index, &node, &slot);
 
+	if (!node) {
+		/*
+		 * We need a node to properly account shadow
+		 * entries. Don't plant any without. XXX
+		 */
+		shadow = NULL;
+	}
+
 	if (shadow) {
 		mapping->nrshadows++;
 		/*
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH stable 4.4 3/4] mm: filemap: fix mapping->nrpages double accounting in fuse
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 1/4] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node Michal Hocko
@ 2016-10-25  7:51 ` Michal Hocko
  2016-10-25  7:51 ` [PATCH stable 4.4 4/4] Using BUG_ON() as an assert() is _never_ acceptable Michal Hocko
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Michal Hocko @ 2016-10-25  7:51 UTC (permalink / raw)
  To: Stable tree
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton, Miklos Szeredi,
	Linus Torvalds, Michal Hocko

From: Johannes Weiner <hannes@cmpxchg.org>

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 <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/filemap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 7ad648c9780c..c588d1222b2a 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -590,7 +590,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH stable 4.4 4/4] Using BUG_ON() as an assert() is _never_ acceptable
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
                   ` (2 preceding siblings ...)
  2016-10-25  7:51 ` [PATCH stable 4.4 3/4] mm: filemap: fix mapping->nrpages double accounting in fuse Michal Hocko
@ 2016-10-25  7:51 ` Michal Hocko
  2016-10-25  7:56 ` [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Michal Hocko @ 2016-10-25  7:51 UTC (permalink / raw)
  To: Stable tree
  Cc: Johannes Weiner, linux-mm, LKML, Linus Torvalds, Miklos Szeredi,
	Andrew Morton, Michal Hocko

From: Linus Torvalds <torvalds@linux-foundation.org>

Commit 21f54ddae449f4bdd9f1498124901d67202243d9 upstream.

That just generally kills the machine, and makes debugging only much
harder, since the traces may long be gone.

Debugging by assert() is a disease.  Don't do it.  If you can continue,
you're much better off doing so with a live machine where you have a
much higher chance that the report actually makes it to the system logs,
rather than result in a machine that is just completely dead.

The only valid situation for BUG_ON() is when continuing is not an
option, because there is massive corruption.  But if you are just
verifying that something is true, you warn about your broken assumptions
(preferably just once), and limp on.

Fixes: 22f2ac51b6d6 ("mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()")
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/swap.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index b28de19aadbf..d8ca2eaa3a8b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -266,7 +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));
+	VM_WARN_ON_ONCE(!workingset_node_pages(node));
 	node->count--;
 }
 
@@ -282,7 +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));
+	VM_WARN_ON_ONCE(!workingset_node_shadows(node));
 	node->count -= 1U << RADIX_TREE_COUNT_SHIFT;
 }
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH stable 4.4 0/4] mm: workingset backports
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
                   ` (3 preceding siblings ...)
  2016-10-25  7:51 ` [PATCH stable 4.4 4/4] Using BUG_ON() as an assert() is _never_ acceptable Michal Hocko
@ 2016-10-25  7:56 ` Michal Hocko
  2016-10-25 14:16 ` Johannes Weiner
  2016-10-26  9:34 ` Greg KH
  6 siblings, 0 replies; 11+ messages in thread
From: Michal Hocko @ 2016-10-25  7:56 UTC (permalink / raw)
  To: Stable tree
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton,
	Antonio SJ Musumeci, Jan Kara, Linus Torvalds, Miklos Szeredi

On Tue 25-10-16 09:51:44, Michal Hocko wrote:
> Hi,
> here is the backport of (hopefully) all workingset related fixes for
> 4.4 kernel. The series has been reviewed by Johannes [1]. The main
> motivation for the backport is 22f2ac51b6d6 ("mm: workingset: fix crash
> in shadow node shrinker caused by replace_page_cache_page()") which is
> a fix for a triggered BUG_ON. This is not sufficient because there are
> follow up fixes which were introduced later.
 
 Ups, forgot to add
[1] http://lkml.kernel.org/r/20161024152605.11707-1-mhocko@kernel.org

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH stable 4.4 0/4] mm: workingset backports
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
                   ` (4 preceding siblings ...)
  2016-10-25  7:56 ` [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
@ 2016-10-25 14:16 ` Johannes Weiner
  2016-10-26  9:34 ` Greg KH
  6 siblings, 0 replies; 11+ messages in thread
From: Johannes Weiner @ 2016-10-25 14:16 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Stable tree, linux-mm, LKML, Andrew Morton, Antonio SJ Musumeci,
	Jan Kara, Linus Torvalds, Michal Hocko, Miklos Szeredi

All 4 backport patches:

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thank you Michal.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH stable 4.4 0/4] mm: workingset backports
  2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
                   ` (5 preceding siblings ...)
  2016-10-25 14:16 ` Johannes Weiner
@ 2016-10-26  9:34 ` Greg KH
  6 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2016-10-26  9:34 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Stable tree, Johannes Weiner, linux-mm, LKML, Andrew Morton,
	Antonio SJ Musumeci, Jan Kara, Linus Torvalds, Michal Hocko,
	Miklos Szeredi

On Tue, Oct 25, 2016 at 09:51:44AM +0200, Michal Hocko wrote:
> Hi,
> here is the backport of (hopefully) all workingset related fixes for
> 4.4 kernel. The series has been reviewed by Johannes [1]. The main
> motivation for the backport is 22f2ac51b6d6 ("mm: workingset: fix crash
> in shadow node shrinker caused by replace_page_cache_page()") which is
> a fix for a triggered BUG_ON. This is not sufficient because there are
> follow up fixes which were introduced later.

Thanks for these, all now queued up.

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node
  2016-10-25  7:51 ` [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node Michal Hocko
@ 2016-10-26 12:45   ` Michal Hocko
  2016-10-26 12:47     ` Michal Hocko
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Hocko @ 2016-10-26 12:45 UTC (permalink / raw)
  To: Stable tree, Greg KH
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton, Linus Torvalds, Jan Kara

Greg,
I do not see this one in the 4.4 queue you have just sent today.

On Tue 25-10-16 09:51:46, Michal Hocko wrote:
> From: Johannes Weiner <hannes@cmpxchg.org>
> 
> 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.
> 
> [hannes@cmpxchg.org: backport for 4.4 stable]
> Fixes: 449dd6984d0e ("mm: keep page cache radix tree nodes in check")
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  mm/filemap.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 4cfe423d3e8a..7ad648c9780c 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -164,6 +164,14 @@ static void page_cache_tree_delete(struct address_space *mapping,
>  
>  	__radix_tree_lookup(&mapping->page_tree, page->index, &node, &slot);
>  
> +	if (!node) {
> +		/*
> +		 * We need a node to properly account shadow
> +		 * entries. Don't plant any without. XXX
> +		 */
> +		shadow = NULL;
> +	}
> +
>  	if (shadow) {
>  		mapping->nrshadows++;
>  		/*
> -- 
> 2.9.3
> 

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node
  2016-10-26 12:45   ` Michal Hocko
@ 2016-10-26 12:47     ` Michal Hocko
  2016-10-26 13:10       ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Hocko @ 2016-10-26 12:47 UTC (permalink / raw)
  To: Stable tree, Greg KH
  Cc: Johannes Weiner, linux-mm, LKML, Andrew Morton, Linus Torvalds, Jan Kara

On Wed 26-10-16 14:45:53, Michal Hocko wrote:
> Greg,
> I do not see this one in the 4.4 queue you have just sent today.

Scratch that. I can see it now on lkml. I just wasn't on the CC so it
hasn't shown up in my inbox.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node
  2016-10-26 12:47     ` Michal Hocko
@ 2016-10-26 13:10       ` Greg KH
  0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2016-10-26 13:10 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Stable tree, Johannes Weiner, linux-mm, LKML, Andrew Morton,
	Linus Torvalds, Jan Kara

On Wed, Oct 26, 2016 at 02:47:53PM +0200, Michal Hocko wrote:
> On Wed 26-10-16 14:45:53, Michal Hocko wrote:
> > Greg,
> > I do not see this one in the 4.4 queue you have just sent today.
> 
> Scratch that. I can see it now on lkml. I just wasn't on the CC so it
> hasn't shown up in my inbox.

Sorry about that, I had applied it earlier in the sequence due to it
being part of the "normal" stable request process.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-10-26 13:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25  7:51 [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
2016-10-25  7:51 ` [PATCH stable 4.4 1/4] mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page() Michal Hocko
2016-10-25  7:51 ` [PATCH stable 4.4 2/4] mm: filemap: don't plant shadow entries without radix tree node Michal Hocko
2016-10-26 12:45   ` Michal Hocko
2016-10-26 12:47     ` Michal Hocko
2016-10-26 13:10       ` Greg KH
2016-10-25  7:51 ` [PATCH stable 4.4 3/4] mm: filemap: fix mapping->nrpages double accounting in fuse Michal Hocko
2016-10-25  7:51 ` [PATCH stable 4.4 4/4] Using BUG_ON() as an assert() is _never_ acceptable Michal Hocko
2016-10-25  7:56 ` [PATCH stable 4.4 0/4] mm: workingset backports Michal Hocko
2016-10-25 14:16 ` Johannes Weiner
2016-10-26  9:34 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).