Skip to content

Commit 5899593

Browse files
jankaratytso
authored andcommitted
ext4: Fix occasional generic/418 failure
Eric has noticed that after pagecache read rework, generic/418 is occasionally failing for ext4 when blocksize < pagesize. In fact, the pagecache rework just made hard to hit race in ext4 more likely. The problem is that since ext4 conversion of direct IO writes to iomap framework (commit 378f32b), we update inode size after direct IO write only after invalidating page cache. Thus if buffered read sneaks at unfortunate moment like: CPU1 - write at offset 1k CPU2 - read from offset 0 iomap_dio_rw(..., IOMAP_DIO_FORCE_WAIT); ext4_readpage(); ext4_handle_inode_extension() the read will zero out tail of the page as it still sees smaller inode size and thus page cache becomes inconsistent with on-disk contents with all the consequences. Fix the problem by moving inode size update into end_io handler which gets called before the page cache is invalidated. Reported-and-tested-by: Eric Whitney <[email protected]> Fixes: 378f32b ("ext4: introduce direct I/O write using iomap infrastructure") CC: [email protected] Signed-off-by: Jan Kara <[email protected]> Acked-by: Dave Chinner <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Theodore Ts'o <[email protected]>
1 parent 5afa7e8 commit 5899593

File tree

1 file changed

+21
-4
lines changed

1 file changed

+21
-4
lines changed

fs/ext4/file.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,15 +371,32 @@ static ssize_t ext4_handle_inode_extension(struct inode *inode, loff_t offset,
371371
static int ext4_dio_write_end_io(struct kiocb *iocb, ssize_t size,
372372
int error, unsigned int flags)
373373
{
374-
loff_t offset = iocb->ki_pos;
374+
loff_t pos = iocb->ki_pos;
375375
struct inode *inode = file_inode(iocb->ki_filp);
376376

377377
if (error)
378378
return error;
379379

380-
if (size && flags & IOMAP_DIO_UNWRITTEN)
381-
return ext4_convert_unwritten_extents(NULL, inode,
382-
offset, size);
380+
if (size && flags & IOMAP_DIO_UNWRITTEN) {
381+
error = ext4_convert_unwritten_extents(NULL, inode, pos, size);
382+
if (error < 0)
383+
return error;
384+
}
385+
/*
386+
* If we are extending the file, we have to update i_size here before
387+
* page cache gets invalidated in iomap_dio_rw(). Otherwise racing
388+
* buffered reads could zero out too much from page cache pages. Update
389+
* of on-disk size will happen later in ext4_dio_write_iter() where
390+
* we have enough information to also perform orphan list handling etc.
391+
* Note that we perform all extending writes synchronously under
392+
* i_rwsem held exclusively so i_size update is safe here in that case.
393+
* If the write was not extending, we cannot see pos > i_size here
394+
* because operations reducing i_size like truncate wait for all
395+
* outstanding DIO before updating i_size.
396+
*/
397+
pos += size;
398+
if (pos > i_size_read(inode))
399+
i_size_write(inode, pos);
383400

384401
return 0;
385402
}

0 commit comments

Comments
 (0)