All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Antonio SJ Musumeci <trapexit@spawn.link>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Dave Jones <davej@codemonkey.org.uk>,
	Oleg Nesterov <oleg@redhat.com>,
	Dave Chinner <david@fromorbit.com>,
	Michal Hocko <mhocko@kernel.org>, Jan Kara <jack@suse.cz>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	stable <stable@vger.kernel.org>
Subject: Re: BUG_ON() in workingset_node_shadows_dec() triggers
Date: Wed, 5 Oct 2016 11:31:35 +0200	[thread overview]
Message-ID: <20161005093135.GB20174@cmpxchg.org> (raw)
In-Reply-To: <20161005092534.GA20174@cmpxchg.org>

On Wed, Oct 05, 2016 at 11:25:34AM +0200, Johannes Weiner wrote:
> Patch below. NOTE: I'm traveling without access to my test rig right
> now and so I have only lightly tested this on my laptop. I'm also
> jetlagged like crazy, so please triple check my thinking. The patch
> does fix the reproducer case and has otherwise been stable here.

There is an issue I spotted in the more recent fuse fix. Same caveat
applies for now regarding testing and brain function, but it should be
a relatively obvious one.

---

>From eaf606185719acaf66412413b7dbac5c683efe11 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Tue, 4 Oct 2016 16:58:06 +0200
Subject: [PATCH] mm: filemap: fix mapping->nrpages double accounting in
 fuse

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>
---
 mm/filemap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 2d0986a64f1f..c17395825650 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -619,7 +619,6 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
 		__delete_from_page_cache(old, NULL);
 		error = page_cache_tree_insert(mapping, new, NULL);
 		BUG_ON(error);
-		mapping->nrpages++;
 
 		/*
 		 * hugetlb pages do not participate in page cache accounting.
-- 
2.10.0

  reply	other threads:[~2016-10-05  9:31 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-04  4:00 BUG_ON() in workingset_node_shadows_dec() triggers Linus Torvalds
2016-10-04  4:07 ` Andrew Morton
2016-10-04  4:12   ` Linus Torvalds
2016-10-04  7:03     ` Raymond Jennings
2016-10-04 16:03       ` Linus Torvalds
2016-10-04  8:02 ` Greg KH
2016-10-04  9:32 ` Johannes Weiner
2016-10-05  1:21   ` Linus Torvalds
2016-10-05  9:25     ` Johannes Weiner
2016-10-05  9:31       ` Johannes Weiner [this message]
2016-10-05 10:40       ` Jan Kara
2016-10-05 16:10       ` Linus Torvalds
2016-10-05 17:00         ` [PATCH] checkpatch: extend BUG warning Joe Perches
2016-10-05 17:00           ` Joe Perches
2016-10-05 17:07           ` Linus Torvalds
2016-10-05  2:43 ` BUG_ON() in workingset_node_shadows_dec() triggers Paul Gortmaker
2016-10-05  3:29   ` Linus Torvalds
2016-10-05  5:44     ` Willy Tarreau
2016-10-05 15:52       ` Linus Torvalds
2016-10-05 19:06         ` Willy Tarreau
2016-10-05 19:18           ` Linus Torvalds
2016-10-05 21:09             ` Willy Tarreau
2016-10-05 21:14             ` Kees Cook
2016-10-05 21:46               ` Linus Torvalds
2016-10-05 22:17                 ` Kees Cook
2016-10-05 22:29                   ` Linus Torvalds
2016-10-06 22:07                     ` Kees Cook
2016-10-06 22:29                       ` Linus Torvalds
2016-10-06 23:05                         ` Kees Cook
2016-10-06 23:59                           ` Linus Torvalds
2016-10-07  5:48                             ` Willy Tarreau
2016-10-07 17:16                               ` Kees Cook
2016-10-07 17:21                                 ` Linus Torvalds
2016-10-07 17:33                                   ` Kees Cook
2016-10-07 18:26                                     ` Willy Tarreau
2016-10-06  1:59     ` Dave Chinner
2016-10-06  2:12       ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161005093135.GB20174@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=davej@codemonkey.org.uk \
    --cc=david@fromorbit.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=oleg@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=trapexit@spawn.link \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.