Skip to content

Commit 1bdec44

Browse files
Hugh Dickinstorvalds
authored andcommitted
tmpfs: fix regressions from wider use of ZERO_PAGE
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]>
1 parent 7fbd166 commit 1bdec44

File tree

2 files changed

+20
-17
lines changed

2 files changed

+20
-17
lines changed

mm/filemap.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,12 +1063,6 @@ void __init pagecache_init(void)
10631063
init_waitqueue_head(&folio_wait_table[i]);
10641064

10651065
page_writeback_init();
1066-
1067-
/*
1068-
* tmpfs uses the ZERO_PAGE for reading holes: it is up-to-date,
1069-
* and splice's page_cache_pipe_buf_confirm() needs to see that.
1070-
*/
1071-
SetPageUptodate(ZERO_PAGE(0));
10721066
}
10731067

10741068
/*

mm/shmem.c

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2513,7 +2513,6 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
25132513
pgoff_t end_index;
25142514
unsigned long nr, ret;
25152515
loff_t i_size = i_size_read(inode);
2516-
bool got_page;
25172516

25182517
end_index = i_size >> PAGE_SHIFT;
25192518
if (index > end_index)
@@ -2570,24 +2569,34 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
25702569
*/
25712570
if (!offset)
25722571
mark_page_accessed(page);
2573-
got_page = true;
2572+
/*
2573+
* Ok, we have the page, and it's up-to-date, so
2574+
* now we can copy it to user space...
2575+
*/
2576+
ret = copy_page_to_iter(page, offset, nr, to);
2577+
put_page(page);
2578+
2579+
} else if (iter_is_iovec(to)) {
2580+
/*
2581+
* Copy to user tends to be so well optimized, but
2582+
* clear_user() not so much, that it is noticeably
2583+
* faster to copy the zero page instead of clearing.
2584+
*/
2585+
ret = copy_page_to_iter(ZERO_PAGE(0), offset, nr, to);
25742586
} else {
2575-
page = ZERO_PAGE(0);
2576-
got_page = false;
2587+
/*
2588+
* But submitting the same page twice in a row to
2589+
* splice() - or others? - can result in confusion:
2590+
* so don't attempt that optimization on pipes etc.
2591+
*/
2592+
ret = iov_iter_zero(nr, to);
25772593
}
25782594

2579-
/*
2580-
* Ok, we have the page, and it's up-to-date, so
2581-
* now we can copy it to user space...
2582-
*/
2583-
ret = copy_page_to_iter(page, offset, nr, to);
25842595
retval += ret;
25852596
offset += ret;
25862597
index += offset >> PAGE_SHIFT;
25872598
offset &= ~PAGE_MASK;
25882599

2589-
if (got_page)
2590-
put_page(page);
25912600
if (!iov_iter_count(to))
25922601
break;
25932602
if (ret < nr) {

0 commit comments

Comments
 (0)