FAILED: patch "[PATCH] mm: filemap: don't plant shadow entries without radix tree" failed to apply to 4.7-stable tree

PATCH [email protected] 2 participants 3 messages Oct 20, 2016 – Oct 26, 2016

Subsystems (4)

Stable branch Memory management Library code Page cache

lore

Share & tools

lore

[email protected]

author maintainer

2016-10-20 14:14 UTC #0 lore raw

The patch below does not apply to the 4.7-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From d3798ae8c6f3767c726403c2ca6ecc317752c9dd Mon Sep 17 00:00:00 2001

From: Johannes Weiner <[email protected]>
Date: Tue, 4 Oct 2016 22:02:08 +0200
Subject: [PATCH] mm: filemap: don't plant shadow entries without radix tree
node

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 <[email protected]>

Reported-and-tested-by Linus Torvalds <[email protected]>

Reviewed-by Jan Kara <[email protected]>

Cc Andrew Morton <[email protected]>

Cc [email protected]

Signed-off-by Linus Torvalds <[email protected]>

include/linux/radix-tree.h +3 -3

index 4c45105dece3..52b97db93830 100644
@@ -280,9 +280,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);
-struct radix_tree_node *radix_tree_replace_clear_tags(
-				struct radix_tree_root *root,
-				unsigned long index, void *entry);
+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);

lib/radix-tree.c +3 ~~-11~~

index 91f0727e3cad..8e6d552c40dd 100644
@@ -1583,15 +1583,10 @@ void *radix_tree_delete(struct radix_tree_root *root, unsigned long index)
 }
 EXPORT_SYMBOL(radix_tree_delete);
 
-struct radix_tree_node *radix_tree_replace_clear_tags(
-			struct radix_tree_root *root,
-			unsigned long index, void *entry)
+void radix_tree_clear_tags(struct radix_tree_root *root,
+			   struct radix_tree_node *node,
+			   void **slot)
 {
-	struct radix_tree_node *node;
-	void **slot;
-
-	__radix_tree_lookup(root, index, &node, &slot);
-
 	if (node) {
 		unsigned int tag, offset = get_slot_offset(node, slot);
 		for (tag = 0; tag < RADIX_TREE_MAX_TAGS; tag++)

@@ -1600,9 +1595,6 @@ struct radix_tree_node *radix_tree_replace_clear_tags(
 		/* Clear root node tags */
 		root->gfp_mask &= __GFP_BITS_MASK;
 	}
-
-	radix_tree_replace_slot(slot, entry);
-	return node;
 }
 
 /**

mm/filemap.c +30 ~~-16~~

69 lines not on this page · raw message

Johannes Weiner

Reviewed-by Jan Kara

2016-10-21 18:21 UTC #1 lore raw

Quoted 5 lines · On Thu, Oct 20, 2016 at 04:15:06PM +0200, [email protected] wrote:

The patch below does not apply to the 4.7-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

---

Quoted 1 line

From 0d5a749ba7faf21f1e2db9fca735d64d2b4b0281 Mon Sep 17 00:00:00 2001

From: Johannes Weiner <[email protected]>
Date: Tue, 4 Oct 2016 22:02:08 +0200
Subject: [PATCH] mm: filemap: don't plant shadow entries without radix tree
node

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

Fixes 449dd6984d0e ("mm: keep page cache radix tree nodes in check")

Signed-off-by Johannes Weiner <[email protected]>

Reported-and-tested-by Linus Torvalds <[email protected]>

Reviewed-by Jan Kara <[email protected]>

Cc Andrew Morton <[email protected]>

Cc [email protected]

Signed-off-by Linus Torvalds <[email protected]>

---
 include/linux/radix-tree.h |  6 +++---
 lib/radix-tree.c           | 14 +++-----------
 mm/filemap.c               | 16 ++++++++++++++--
 3 files changed, 20 insertions(+), 16 deletions(-)

include/linux/radix-tree.h +3 -3

index eca6f626c16e..82ac7c2e084f 100644
@@ -280,9 +280,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);
-struct radix_tree_node *radix_tree_replace_clear_tags(
-				struct radix_tree_root *root,
-				unsigned long index, void *entry);
+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);

lib/radix-tree.c +3 ~~-11~~

index 8b7d8459bb9d..de24f28deff7 100644
@@ -1529,15 +1529,10 @@ void *radix_tree_delete(struct radix_tree_root *root, unsigned long index)
 }
 EXPORT_SYMBOL(radix_tree_delete);
 
-struct radix_tree_node *radix_tree_replace_clear_tags(
-			struct radix_tree_root *root,
-			unsigned long index, void *entry)
+void radix_tree_clear_tags(struct radix_tree_root *root,
+			   struct radix_tree_node *node,
+			   void **slot)
 {
-	struct radix_tree_node *node;
-	void **slot;
-
-	__radix_tree_lookup(root, index, &node, &slot);
-
 	if (node) {
 		unsigned int tag, offset = get_slot_offset(node, slot);
 		for (tag = 0; tag < RADIX_TREE_MAX_TAGS; tag++)

@@ -1546,9 +1541,6 @@ struct radix_tree_node *radix_tree_replace_clear_tags(
 		/* Clear root node tags */
 		root->gfp_mask &= __GFP_BITS_MASK;
 	}
-
-	radix_tree_replace_slot(slot, entry);
-	return node;
 }
 
 /**

mm/filemap.c +14 -2

index 20f3b1f33f0e..6c7beb057683 100644
@@ -114,11 +114,23 @@ static void page_cache_tree_delete(struct address_space *mapping,
 				   struct page *page, void *shadow)
 {
 	struct radix_tree_node *node;
+	void **slot;
 
 	VM_BUG_ON(!PageLocked(page));
 
-	node = radix_tree_replace_clear_tags(&mapping->page_tree, page->index,
-								shadow);
+	__radix_tree_lookup(&mapping->page_tree, page->index, &node, &slot);
+
+	radix_tree_clear_tags(&mapping->page_tree, node, slot);
+
+	if (!node) {
+		/*
+		 * We need a node to properly account shadow
+		 * entries. Don't plant any without. XXX
+		 */
+		shadow = NULL;
+	}
+
+	radix_tree_replace_slot(slot, shadow);
 
 	if (shadow) {
 		mapping->nrexceptional++;

Signature · 1 line

2.10.0

Greg KH

author maintainer

2016-10-26 08:39 UTC #2 lore raw

Quoted 9 lines · Johannes Weiner

Quoted 5 lines · On Thu, Oct 20, 2016 at 04:15:06PM +0200, [email protected] wrote:
The patch below does not apply to the 4.7-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
---

Quoted 7 lines

Quoted 1 line
From 0d5a749ba7faf21f1e2db9fca735d64d2b4b0281 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <[email protected]>
Date: Tue, 4 Oct 2016 22:02:08 +0200
Subject: [PATCH] mm: filemap: don't plant shadow entries without radix tree
node
commit d3798ae8c6f3767c726403c2ca6ecc317752c9dd upstream.

Thanks for this, but 4.7.y is now end-of-life :(

greg k-h