linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: dhowells@redhat.com, Jens Axboe <axboe@kernel.dk>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Matthew Wilcox <willy@infradead.org>, Jan Kara <jack@suse.cz>,
	Jeff Layton <jlayton@kernel.org>,
	David Hildenbrand <david@redhat.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Logan Gunthorpe <logang@deltatee.com>,
	Hillf Danton <hdanton@sina.com>,
	linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Daniel Golle <daniel@makrotopia.org>,
	Guenter Roeck <groeck7@gmail.com>, Christoph Hellwig <hch@lst.de>,
	John Hubbard <jhubbard@nvidia.com>,
	Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH v17 03/14] shmem: Implement splice-read
Date: Wed, 08 Mar 2023 23:42:32 +0000	[thread overview]
Message-ID: <2139517.1678318952@warthog.procyon.org.uk> (raw)
In-Reply-To: <CAHk-=wjYR3h5Q-_i3Q2Et=P8WsrjwNA20fYpEQf9nafHwBNALA@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> I get the feeling that the zeropage case just isn't so important that we'd
> need to duplicate filemap_splice_read() just for that, and I think that the
> code should either
> 
>  (a) just make a silly "read_folio()" for shmfs that just clears the page.
> 
>      Ugly but maybe simple and not horrid?

The problem with that is that once a page is in the pagecache attached to a
shmem file, we can't get rid of it without deleting or truncating the file...
At least, I think that the case.  For all other regular filesystems, a page
full of zeros can be flushed/discarded by the LRU.

shmem also has its own function for allocating folios in its pagecache, the
caller of ->read_folio() would probably have to use that.

>  (b) teach filemap_splice_read() that a NULL 'read_folio' function
> means "use the zero page"
> 
>      That might not be splice() itself, but maybe in
> filemap_get_pages() or something.

It would require some special handling in filemap_get_pages() - and/or
probably better filemap_splice_read() since, for shmem, it's only relevant to
splice.  An additional flag could be added to filemap_get_pages() to tell it
to stop at a hole in the pagecache rather than invoking readahead.
filemap_splice_read() would then need to examine the pagecache to work out how
big the hole is and insert the appropriate number of zeropages before calling
back into filemap_get_pages() again.  Possibly it could use SEEK_DATA.

> or
> 
>  (c) go even further, and teach read_folio() in general about file
> holes, and allow *any* filesystem to read zeroes that way in general
> without creating a folio for it.

Nice idea, but we'd need a way to store a "negative" marker (as opposed to
"unknown") in the pagecache for the filemap code to be able to use it.  This
sort of thing might become easier if xarray gets switched to a maple tree
implementation as that would better allow for caching of a known file hole of
arbitrary size with a single entry.

But for the moment, the filemap code would have to jump through a filesystem's
->readahead or ->read_folio vectors to work out if there's a hole there or not
- but in both cases it must already have allocated the pages it wants to
query.

David



  parent reply	other threads:[~2023-03-08 23:42 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-08 16:52 [PATCH v17 00/14] splice, block: Use page pinning and kill ITER_PIPE David Howells
2023-03-08 16:52 ` [PATCH v17 01/14] splice: Clean up direct_splice_read() a bit David Howells
2023-03-14 17:30   ` Christoph Hellwig
2023-03-08 16:52 ` [PATCH v17 02/14] splice: Make do_splice_to() generic and export it David Howells
2023-03-14 17:31   ` Christoph Hellwig
2023-03-14 21:15   ` David Howells
2023-03-15 16:34   ` [RFC PATCH] splice: Convert longs and some ints into ssize_t David Howells
2023-03-08 16:52 ` [PATCH v17 03/14] shmem: Implement splice-read David Howells
2023-03-08 22:39   ` Linus Torvalds
2023-03-14 16:42     ` Matthew Wilcox
2023-03-14 18:02       ` Linus Torvalds
2023-03-14 20:08         ` Matthew Wilcox
2023-03-14 18:26       ` David Howells
2023-03-14 19:07         ` Linus Torvalds
2023-03-14 19:09           ` Linus Torvalds
2023-03-14 21:50         ` David Howells
2023-03-08 23:42   ` David Howells [this message]
2023-03-08 16:52 ` [PATCH v17 04/14] overlayfs: " David Howells
2023-03-08 16:52 ` [PATCH v17 05/14] coda: " David Howells
2023-03-13 13:28   ` Jan Harkes
2023-03-08 16:52 ` [PATCH v17 06/14] tty, proc, kernfs, random: Use direct_splice_read() David Howells
2023-03-08 16:52 ` [PATCH v17 07/14] splice: Do splice read from a file without using ITER_PIPE David Howells
2023-03-14 17:32   ` Christoph Hellwig
2023-03-14 21:52   ` David Howells
2023-03-08 16:52 ` [PATCH v17 08/14] iov_iter: Kill ITER_PIPE David Howells
2023-03-08 16:52 ` [PATCH v17 09/14] iomap: Don't get an reference on ZERO_PAGE for direct I/O block zeroing David Howells
2023-03-08 21:08   ` Dave Chinner
2023-03-14 17:33   ` Christoph Hellwig
2023-03-08 16:52 ` [PATCH v17 10/14] block: Fix bio_flagged() so that gcc can better optimise it David Howells
2023-03-08 16:52 ` [PATCH v17 11/14] block: Replace BIO_NO_PAGE_REF with BIO_PAGE_REFFED with inverted logic David Howells
2023-03-08 16:52 ` [PATCH v17 12/14] block: Add BIO_PAGE_PINNED and associated infrastructure David Howells
2023-03-08 16:52 ` [PATCH v17 13/14] block: Convert bio_iov_iter_get_pages to use iov_iter_extract_pages David Howells
2023-03-08 16:52 ` [PATCH v17 14/14] block: convert bio_map_user_iov " David Howells

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=2139517.1678318952@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=daniel@makrotopia.org \
    --cc=david@redhat.com \
    --cc=groeck7@gmail.com \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=hdanton@sina.com \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=jgg@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=jlayton@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=logang@deltatee.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.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