linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Yang Shi <shy828301@gmail.com>
Cc: Hugh Dickins <hughd@google.com>,
	cfijalkovich@google.com, song@kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Hao Sun <sunhao.th@gmail.com>, Linux MM <linux-mm@kvack.org>,
	Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Song Liu <songliubraving@fb.com>
Subject: Re: [PATCH] fs: buffer: check huge page size instead of single page for invalidatepage
Date: Mon, 20 Sep 2021 22:50:27 +0100	[thread overview]
Message-ID: <YUkCI2I085Sos/64@casper.infradead.org> (raw)
In-Reply-To: <CAHbLzkrPDDoOsPXQD3Y3Kbmex4ptYH+Ad_P1Ds_ateWb+65Rng@mail.gmail.com>

On Mon, Sep 20, 2021 at 02:23:41PM -0700, Yang Shi wrote:
> On Sun, Sep 19, 2021 at 7:41 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Fri, Sep 17, 2021 at 05:07:03PM -0700, Yang Shi wrote:
> > > > The debugging showed the page passed to invalidatepage is a huge page
> > > > and the length is the size of huge page instead of single page due to
> > > > read only FS THP support.  But block_invalidatepage() would throw BUG if
> > > > the size is greater than single page.
> >
> > Things have already gone wrong before we get to this point.  See
> > do_dentry_open().  You aren't supposed to be able to get a writable file
> > descriptor on a file which has had huge pages added to the page cache
> > without the filesystem's knowledge.  That's the problem that needs to
> > be fixed.
> 
> I don't quite understand your point here. Do you mean do_dentry_open()
> should fail for such cases instead of truncating the page cache?

No, do_dentry_open() should have truncated the page cache when it was
called and found that there were THPs in the cache.  Then khugepaged
should see that someone has the file open for write and decline to create
new THPs.  So it shouldn't be possible to get here with THPs in the cache.

  reply	other threads:[~2021-09-21  2:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-17 20:57 [PATCH] fs: buffer: check huge page size instead of single page for invalidatepage Yang Shi
2021-09-18  0:07 ` Yang Shi
2021-09-19 14:40   ` Matthew Wilcox
2021-09-20 21:23     ` Yang Shi
2021-09-20 21:50       ` Matthew Wilcox [this message]
2021-09-20 22:35         ` Yang Shi
2021-10-11 19:57           ` Yang Shi
2021-10-20 23:38             ` Yang Shi
2021-10-20 23:49               ` Matthew Wilcox
2021-10-21  0:24                 ` Yang Shi
2021-10-21  1:36                   ` Matthew Wilcox

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=YUkCI2I085Sos/64@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=cfijalkovich@google.com \
    --cc=hughd@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.com \
    --cc=song@kernel.org \
    --cc=songliubraving@fb.com \
    --cc=sunhao.th@gmail.com \
    /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 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).