[5/6] xfs: skip CoW writes past EOF when writeback races with truncate

Commit Message

Darrick J. Wong Jan. 26, 2018, 2:05 a.m. UTC

From: Darrick J. Wong <[e-mail]>

Every so often we blow the ASSERT(type != XFS_IO_COW) in xfs_map_blocks
when running fsstress, as we do in generic/269.  The cause of this is
writeback racing with truncate -- writeback doesn't take the iolock, so
truncate can sneak in to decrease i_size and truncate page cache while
writeback is gathering buffer heads to schedule writeout.

If we hit this race on a block that has a CoW mapping, we'll get a valid
imap from the CoW fork but the reduced i_size trims the mapping to zero
length (which makes it invalid), so we call xfs_map_blocks to try again.
This doesn't do much anyway, since any mapping we get out of that will
also be invalid, so we might as well skip the assert and just stop.

Signed-off-by: Darrick J. Wong <[e-mail]>
---
 fs/xfs/xfs_aops.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)



--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Christoph Hellwig Jan. 26, 2018, 12:21 p.m. UTC | #1

> +	/*
> +	 * Apparently truncate can race with writeback since writeback doesn't

The apparently here looks rather snarky, I'd remove it.

> +	 * take the iolock and truncate decreases the file size before it
> +	 * starts truncating the pages between new_size and old_size.
> +	 * Therefore, we can end up in the situation where writeback gets a
> +	 * CoW fork mapping but the truncate makes the mapping invalid and we
> +	 * end up in here trying to get a new mapping.  Bail out here so that
> +	 * we simply never get a valid mapping and so we drop the write
> +	 * altogether.  The page truncation will kill the contents anyway.
> +	 */
> +	if (type == XFS_IO_COW && offset > i_size_read(inode))
> +		return 0;

Otherwise this looks fine:

Reviewed-by: Christoph Hellwig <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 2e094c7..2ebc42e 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -390,6 +390,19 @@  xfs_map_blocks(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
+	/*
+	 * Apparently truncate can race with writeback since writeback doesn't
+	 * take the iolock and truncate decreases the file size before it
+	 * starts truncating the pages between new_size and old_size.
+	 * Therefore, we can end up in the situation where writeback gets a
+	 * CoW fork mapping but the truncate makes the mapping invalid and we
+	 * end up in here trying to get a new mapping.  Bail out here so that
+	 * we simply never get a valid mapping and so we drop the write
+	 * altogether.  The page truncation will kill the contents anyway.
+	 */
+	if (type == XFS_IO_COW && offset > i_size_read(inode))
+		return 0;
+
 	ASSERT(type != XFS_IO_COW);
 	if (type == XFS_IO_UNWRITTEN)
 		bmapi_flags |= XFS_BMAPI_IGSTATE;