Skip to content

Commit 56548b6

Browse files
Brian Fostergregkh
authored andcommitted
xfs: don't BUG() on mixed direct and mapped I/O
commit 04197b3 upstream. We've had reports of generic/095 causing XFS to BUG() in __xfs_get_blocks() due to the existence of delalloc blocks on a direct I/O read. generic/095 issues a mix of various types of I/O, including direct and memory mapped I/O to a single file. This is clearly not supported behavior and is known to lead to such problems. E.g., the lack of exclusion between the direct I/O and write fault paths means that a write fault can allocate delalloc blocks in a region of a file that was previously a hole after the direct read has attempted to flush/inval the file range, but before it actually reads the block mapping. In turn, the direct read discovers a delalloc extent and cannot proceed. While the appropriate solution here is to not mix direct and memory mapped I/O to the same regions of the same file, the current BUG_ON() behavior is probably overkill as it can crash the entire system. Instead, localize the failure to the I/O in question by returning an error for a direct I/O that cannot be handled safely due to delalloc blocks. Be careful to allow the case of a direct write to post-eof delalloc blocks. This can occur due to speculative preallocation and is safe as post-eof blocks are not accompanied by dirty pages in pagecache (conversely, preallocation within eof must have been zeroed, and thus dirtied, before the inode size could have been increased beyond said blocks). Finally, provide an additional warning if a direct I/O write occurs while the file is memory mapped. This may not catch all problematic scenarios, but provides a hint that some known-to-be-problematic I/O methods are in use. Signed-off-by: Brian Foster <[email protected]> Reviewed-by: Dave Chinner <[email protected]> Signed-off-by: Dave Chinner <[email protected]> Signed-off-by: Nikolay Borisov <[email protected]> Acked-by: Darrick J. Wong <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent eb0760d commit 56548b6

File tree

1 file changed

+20
-1
lines changed

1 file changed

+20
-1
lines changed

fs/xfs/xfs_aops.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1426,6 +1426,26 @@ __xfs_get_blocks(
14261426
if (error)
14271427
goto out_unlock;
14281428

1429+
/*
1430+
* The only time we can ever safely find delalloc blocks on direct I/O
1431+
* is a dio write to post-eof speculative preallocation. All other
1432+
* scenarios are indicative of a problem or misuse (such as mixing
1433+
* direct and mapped I/O).
1434+
*
1435+
* The file may be unmapped by the time we get here so we cannot
1436+
* reliably fail the I/O based on mapping. Instead, fail the I/O if this
1437+
* is a read or a write within eof. Otherwise, carry on but warn as a
1438+
* precuation if the file happens to be mapped.
1439+
*/
1440+
if (direct && imap.br_startblock == DELAYSTARTBLOCK) {
1441+
if (!create || offset < i_size_read(VFS_I(ip))) {
1442+
WARN_ON_ONCE(1);
1443+
error = -EIO;
1444+
goto out_unlock;
1445+
}
1446+
WARN_ON_ONCE(mapping_mapped(VFS_I(ip)->i_mapping));
1447+
}
1448+
14291449
/* for DAX, we convert unwritten extents directly */
14301450
if (create &&
14311451
(!nimaps ||
@@ -1525,7 +1545,6 @@ __xfs_get_blocks(
15251545
set_buffer_new(bh_result);
15261546

15271547
if (imap.br_startblock == DELAYSTARTBLOCK) {
1528-
BUG_ON(direct);
15291548
if (create) {
15301549
set_buffer_uptodate(bh_result);
15311550
set_buffer_mapped(bh_result);

0 commit comments

Comments
 (0)