Re: [PATCH v17 03/14] shmem: Implement splice-read

[David Howells]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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