linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-xfs@vger.kernel.org
Subject: Re: Splitting a THP beyond EOF
Date: Thu, 22 Oct 2020 00:04:22 +0100	[thread overview]
Message-ID: <20201021230422.GP20115@casper.infradead.org> (raw)
In-Reply-To: <20201021221435.GR7391@dread.disaster.area>

On Thu, Oct 22, 2020 at 09:14:35AM +1100, Dave Chinner wrote:
> On Tue, Oct 20, 2020 at 11:53:31PM +0100, Matthew Wilcox wrote:
> > True, we don't _have to_ split THP on holepunch/truncation/... but it's
> > a better implementation to free pages which cover blocks that no longer
> > have data associated with them.
> 
> "Better" is a very subjective measure. What numbers do you have
> to back that up?

None.  When we choose to use a THP, we're choosing to treat a chunk
of a file as a single unit for the purposes of tracking dirtiness,
age, membership of the workingset, etc.  We're trading off reduced
precision for reduced overhead; just like the CPU tracks dirtiness on
a cacheline basis instead of at byte level.

So at some level, we've making the assumption that this 128kB THP is
all one thingand it should be tracked together.  But the user has just
punched a hole in it.  I can think of no stronger signal to say "The
piece before this hole, the piece I just got rid of and the piece after
this are three separate pieces of the file".

If I could split them into pieces that weren't single pages, I would.
Zi Yan has a patch to do just that, and I'm very much looking forward
to that being merged.  But saying "Oh, this is quite small, I'll keep
the rest of the THP together" is conceptually wrong.

> > Splitting the page instead of throwing it away makes sense once we can
> > transfer the Uptodate bits to each subpage.  If we don't have that,
> > it doesn't really matter which we do.
> 
> Sounds like more required functionality...

I'm not saying that my patchset is the last word and there will be no
tweaking.  I'm saying I think it's good enough, an improvement on the
status quo, and it's better to merge it for 5.11 than to keep it out of
tree for another three months while we tinker with improving it.

Do you disagree?

  reply	other threads:[~2020-10-21 23:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20  1:43 Splitting a THP beyond EOF Matthew Wilcox
2020-10-20  4:59 ` Dave Chinner
2020-10-20 11:21   ` Matthew Wilcox
2020-10-20 21:16     ` Dave Chinner
2020-10-20 22:53       ` Matthew Wilcox
2020-10-21 22:14         ` Dave Chinner
2020-10-21 23:04           ` Matthew Wilcox [this message]
2020-10-27  5:31             ` Dave Chinner
2020-10-27 12:14               ` Matthew Wilcox
2020-10-20 14:26 ` Matthew Wilcox
2020-10-20 14:32 ` Chris Mason
2020-10-21  0:23   ` 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=20201021230422.GP20115@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=david@fromorbit.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    /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).