Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 00/36] Large pages in the page cache
Date: Fri, 22 May 2020 08:49:06 +1000	[thread overview]
Message-ID: <20200521224906.GU2005@dread.disaster.area> (raw)
In-Reply-To: <20200515131656.12890-1-willy@infradead.org>

On Fri, May 15, 2020 at 06:16:20AM -0700, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> 
> This patch set does not pass xfstests.  Test at your own risk.  It is
> based on the readahead rewrite which is in Andrew's tree.  I've fixed a
> lot of issues in the last two weeks, but generic/013 will still crash it.
> 
> The primary idea here is that a large part of the overhead in dealing
> with individual pages is that there's just so darned many of them.
> We would be better off dealing with fewer, larger pages, even if they
> don't get to be the size necessary for the CPU to use a larger TLB entry.

Ok, so the main issue I have with the filesystem/iomap side of
things is that it appears to be adding "transparent huge page"
awareness to the filesysetm code, not "large page support".

For people that aren't aware of the difference between the
transparent huge and and a normal compound page (e.g. I have no idea
what the difference is), this is likely to cause problems,
especially as you haven't explained at all in this description why
transparent huge pages are being used rather than bog standard
compound pages.

And, really, why should iomap or the filesystems care if the large
page is a THP or just a high order compound page? The interface
for operating on these things at the page cache level should be the
same. We already have page_size() and friends for operating on
high order compound pages, yet the iomap stuff has this new
thp_size() function instead of just using page_size(). THis is going
to lead to confusion and future bugs when people who don't know the
difference use the wrong page size function in their filesystem
code.

So, really, the "large page" API presented to the filesystems via
the page cache needs to be unified. Having to use compound_*() in
some places, thp_* in others, then page_* and Page*, not to mention
hpage_* just so that we can correctly support "large pages" is a
total non-starter.

Hence I'd suggest that this patch set needs to start by "hiding" all
the differences between different types of pages behind a unified,
consistent API, then it can introduce large page support into code
outside the mm/ infrastructure via that unified API. I don't care
what that API looks like so long as it is clear, consistenti, well
documented and means filesystem developers don't need to know
anything about how the page (large or not) is managed by the mm
subsystem.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2020-05-21 22:49 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 13:16 Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 01/36] mm: Move PageDoubleMap bit Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 02/36] mm: Simplify PageDoubleMap with PF_SECOND policy Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 03/36] mm: Allow hpages to be arbitrary order Matthew Wilcox
2020-05-28 14:19   ` Zi Yan
2020-05-15 13:16 ` [PATCH v4 04/36] mm: Introduce thp_size Matthew Wilcox
2020-05-15 13:38   ` David Hildenbrand
2020-05-15 13:16 ` [PATCH v4 05/36] mm: Introduce thp_order Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 06/36] mm: Introduce offset_in_thp Matthew Wilcox
2020-05-15 13:39   ` David Hildenbrand
2020-05-22 17:15   ` Kirill A. Shutemov
2020-05-29 12:59     ` Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 07/36] fs: Add a filesystem flag for large pages Matthew Wilcox
2020-05-21 21:55   ` Dave Chinner
2020-05-21 23:29     ` Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 08/36] fs: Do not update nr_thps for large page mappings Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 09/36] fs: Introduce i_blocks_per_page Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 10/36] fs: Make page_mkwrite_check_truncate thp-aware Matthew Wilcox
2020-05-21 22:01   ` Dave Chinner
2020-05-21 23:30     ` Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 11/36] fs: Support THPs in zero_user_segments Matthew Wilcox
2020-05-25  4:55   ` Kirill A. Shutemov
2020-05-15 13:16 ` [PATCH v4 12/36] bio: Add bio_for_each_thp_segment_all Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 13/36] iomap: Support arbitrarily many blocks per page Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 14/36] iomap: Support large pages in iomap_adjust_read_range Matthew Wilcox
2020-05-21 22:24   ` Dave Chinner
2020-05-21 23:39     ` Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 15/36] iomap: Support large pages in read paths Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 16/36] iomap: Support large pages in write paths Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 17/36] iomap: Inline data shouldn't see large pages Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 18/36] iomap: Handle tail pages in iomap_page_mkwrite Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 19/36] xfs: Support large pages Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 20/36] mm: Make prep_transhuge_page return its argument Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 21/36] mm: Add __page_cache_alloc_order Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 22/36] mm: Allow large pages to be added to the page cache Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 23/36] mm: Allow large pages to be removed from " Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 24/36] mm: Remove page fault assumption of compound page size Matthew Wilcox
2020-05-25  4:59   ` Kirill A. Shutemov
2020-05-15 13:16 ` [PATCH v4 25/36] mm: Fix total_mapcount assumption of " Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 26/36] mm: Avoid splitting large pages Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 27/36] mm: Fix truncation for pages of arbitrary size Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 28/36] mm: Support storing shadow entries for large pages Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 29/36] mm: Support retrieving tail pages from the page cache Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 30/36] mm: Support tail pages in wait_for_stable_page Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 31/36] mm: Add DEFINE_READAHEAD Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 32/36] mm: Make page_cache_readahead_unbounded take a readahead_control Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 33/36] mm: Make __do_page_cache_readahead " Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 34/36] mm: Allow PageReadahead to be set on head pages Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 35/36] mm: Add large page readahead Matthew Wilcox
2020-05-15 13:16 ` [PATCH v4 36/36] mm: Align THP mappings for non-DAX Matthew Wilcox
2020-05-26 22:05   ` William Kucharski
2020-05-26 22:20     ` Matthew Wilcox
2020-05-21 22:49 ` Dave Chinner [this message]
2020-05-22  0:04   ` [PATCH v4 00/36] Large pages in the page cache Matthew Wilcox
2020-05-22  2:57     ` Dave Chinner
2020-05-22  3:05       ` Matthew Wilcox
2020-05-25 23:07         ` Dave Chinner
2020-05-26  1:21           ` Matthew Wilcox
2020-05-28 11:00 ` William Kucharski

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=20200521224906.GU2005@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH v4 00/36] Large pages in the page cache' \
    /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

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).