Skip to content

Commit

Permalink
tmpfs: fix regressions from wider use of ZERO_PAGE
Browse files Browse the repository at this point in the history
Chuck Lever reported fsx-based xfstests generic 075 091 112 127 failing
when 5.18-rc1 NFS server exports tmpfs: bisected to recent tmpfs change.

Whilst nfsd_splice_action() does contain some questionable handling of
repeated pages, and Chuck was able to work around there, history from
Mark Hemment makes clear that there might be similar dangers elsewhere:
it was not a good idea for me to pass ZERO_PAGE down to unknown actors.

Revert shmem_file_read_iter() to using ZERO_PAGE for holes only when
iter_is_iovec(); in other cases, use the more natural iov_iter_zero()
instead of copy_page_to_iter().

We would use iov_iter_zero() throughout, but the x86 clear_user() is not
nearly so well optimized as copy to user (dd of 1T sparse tmpfs file
takes 57 seconds rather than 44 seconds).

And now pagecache_init() does not need to SetPageUptodate(ZERO_PAGE(0)):
which had caused boot failure on arm noMMU STM32F7 and STM32H7 boards

Link: https://lkml.kernel.org/r/[email protected]
Fixes: 56a8c8e ("tmpfs: do not allocate pages on read")
Signed-off-by: Hugh Dickins <[email protected]>
Reported-by: Patrice CHOTARD <[email protected]>
Reported-by: Chuck Lever III <[email protected]>
Tested-by: Chuck Lever III <[email protected]>
Cc: Mark Hemment <[email protected]>
Cc: Patrice CHOTARD <[email protected]>
Cc: Mikulas Patocka <[email protected]>
Cc: Lukas Czerner <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: "Darrick J. Wong" <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Hugh Dickins authored and torvalds committed Apr 15, 2022
1 parent 7fbd166 commit 1bdec44
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 17 deletions.
6 changes: 0 additions & 6 deletions mm/filemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1063,12 +1063,6 @@ void __init pagecache_init(void)
init_waitqueue_head(&folio_wait_table[i]);

page_writeback_init();

/*
* tmpfs uses the ZERO_PAGE for reading holes: it is up-to-date,
* and splice's page_cache_pipe_buf_confirm() needs to see that.
*/
SetPageUptodate(ZERO_PAGE(0));
}

/*
Expand Down
31 changes: 20 additions & 11 deletions mm/shmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -2513,7 +2513,6 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
pgoff_t end_index;
unsigned long nr, ret;
loff_t i_size = i_size_read(inode);
bool got_page;

end_index = i_size >> PAGE_SHIFT;
if (index > end_index)
Expand Down Expand Up @@ -2570,24 +2569,34 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
*/
if (!offset)
mark_page_accessed(page);
got_page = true;
/*
* Ok, we have the page, and it's up-to-date, so
* now we can copy it to user space...
*/
ret = copy_page_to_iter(page, offset, nr, to);
put_page(page);

} else if (iter_is_iovec(to)) {
/*
* Copy to user tends to be so well optimized, but
* clear_user() not so much, that it is noticeably
* faster to copy the zero page instead of clearing.
*/
ret = copy_page_to_iter(ZERO_PAGE(0), offset, nr, to);
} else {
page = ZERO_PAGE(0);
got_page = false;
/*
* But submitting the same page twice in a row to
* splice() - or others? - can result in confusion:
* so don't attempt that optimization on pipes etc.
*/
ret = iov_iter_zero(nr, to);
}

/*
* Ok, we have the page, and it's up-to-date, so
* now we can copy it to user space...
*/
ret = copy_page_to_iter(page, offset, nr, to);
retval += ret;
offset += ret;
index += offset >> PAGE_SHIFT;
offset &= ~PAGE_MASK;

if (got_page)
put_page(page);
if (!iov_iter_count(to))
break;
if (ret < nr) {
Expand Down

0 comments on commit 1bdec44

Please sign in to comment.