xfs: fix shared extent data corruption due to missing cow reservation

Page writeback indirectly handles shared extents via the existence of overlapping COW fork blocks. If COW fork blocks exist, writeback always performs the associated copy-on-write regardless if the underlying blocks are actually shared. If the blocks are shared, then overlapping COW fork blocks must always exist.

fstests shared/010 reproduces a case where a buffered write occurs over a shared block without performing the requisite COW fork reservation. This ultimately causes writeback to the shared extent and data corruption that is detected across md5 checks of the filesystem across a mount cycle.

The problem occurs when a buffered write lands over a shared extent that crosses an extent size hint boundary and that also happens to have a partial COW reservation that doesn't cover the start and end blocks of the data fork extent.

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Update xfs_reflink_reserve_cow() to accommodate the fact that a delalloc allocation request may not fully cover the extent in the data fork. Trim the data fork extent appropriately, just as is done for shared extent boundaries and/or existing COW reservations that happen to overlap the start of the data fork extent. This prevents shared/010 failures due to data corruption on reflink enabled filesystems.

Signed-off-by Brian Foster <[email protected]>

---

This is not fully tested yet beyond verification that it solves the problem reproduced by shared/010. I'll be running more tests today, but I'm sending sooner for review and testing due to the nature of the problem and the fact that it's a fairly isolated change. I'll follow up if I discover any resulting regressions..

Brian

 fs/xfs/xfs_reflink.c | 1 +
 1 file changed, 1 insertion(+)

fs/xfs/xfs_reflink.c +1 -0

index ecdb086bc23e..c56bdbfcf7ae 100644
@@ -296,6 +296,7 @@ xfs_reflink_reserve_cow(
 	if (error)
 		return error;
 
+	xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
 	trace_xfs_reflink_cow_alloc(ip, &got);
 	return 0;
 }

Signature · 1 line

2.17.2

I ran this through my overnight tests and saw no adverse effects, though Dave was complaining yesterday about continuing generic/091 corruptions (which I didn't see with this patch applied...)

Quoted 43 lines · Brian Foster

Page writeback indirectly handles shared extents via the existence of overlapping COW fork blocks. If COW fork blocks exist, writeback always performs the associated copy-on-write regardless if the underlying blocks are actually shared. If the blocks are shared, then overlapping COW fork blocks must always exist.

fstests shared/010 reproduces a case where a buffered write occurs over a shared block without performing the requisite COW fork reservation. This ultimately causes writeback to the shared extent and data corruption that is detected across md5 checks of the filesystem across a mount cycle.

The problem occurs when a buffered write lands over a shared extent that crosses an extent size hint boundary and that also happens to have a partial COW reservation that doesn't cover the start and end blocks of the data fork extent.

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Update xfs_reflink_reserve_cow() to accommodate the fact that a delalloc allocation request may not fully cover the extent in the data fork. Trim the data fork extent appropriately, just as is done for shared extent boundaries and/or existing COW reservations that happen to overlap the start of the data fork extent. This prevents shared/010 failures due to data corruption on reflink enabled filesystems.

Signed-off-by Brian Foster <[email protected]>

---

Quoted 5 lines

This is not fully tested yet beyond verification that it solves the problem reproduced by shared/010. I'll be running more tests today, but I'm sending sooner for review and testing due to the nature of the problem and the fact that it's a fairly isolated change. I'll follow up if I discover any resulting regressions..

Did you find any regressions?

I ran this through my overnight tests and saw no adverse effects, though Dave was complaining yesterday about continuing generic/091 corruptions (which I didn't see with this patch applied...)

Anyway it looks reasonable to me...

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 20 lines

Brian

 fs/xfs/xfs_reflink.c | 1 +
 1 file changed, 1 insertion(+)

fs/xfs/xfs_reflink.c +1 -0

index ecdb086bc23e..c56bdbfcf7ae 100644
@@ -296,6 +296,7 @@ xfs_reflink_reserve_cow(
 	if (error)
 		return error;
 
+	xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
 	trace_xfs_reflink_cow_alloc(ip, &got);
 	return 0;
 }

Signature · 1 line

2.17.2

Quoted 40 lines · Brian Foster

Page writeback indirectly handles shared extents via the existence of overlapping COW fork blocks. If COW fork blocks exist, writeback always performs the associated copy-on-write regardless if the underlying blocks are actually shared. If the blocks are shared, then overlapping COW fork blocks must always exist.

fstests shared/010 reproduces a case where a buffered write occurs over a shared block without performing the requisite COW fork reservation. This ultimately causes writeback to the shared extent and data corruption that is detected across md5 checks of the filesystem across a mount cycle.

The problem occurs when a buffered write lands over a shared extent that crosses an extent size hint boundary and that also happens to have a partial COW reservation that doesn't cover the start and end blocks of the data fork extent.

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Update xfs_reflink_reserve_cow() to accommodate the fact that a delalloc allocation request may not fully cover the extent in the data fork. Trim the data fork extent appropriately, just as is done for shared extent boundaries and/or existing COW reservations that happen to overlap the start of the data fork extent. This prevents shared/010 failures due to data corruption on reflink enabled filesystems.

Quoted 8 lines

Signed-off-by Brian Foster <[email protected]>

---

This is not fully tested yet beyond verification that it solves the problem reproduced by shared/010. I'll be running more tests today, but I'm sending sooner for review and testing due to the nature of the problem and the fact that it's a fairly isolated change. I'll follow up if I discover any resulting regressions..

Did you find any regressions?

I ran this through my overnight tests and saw no adverse effects, though Dave was complaining yesterday about continuing generic/091 corruptions (which I didn't see with this patch applied...)

Quoted 52 lines · Darrick J. Wong

Quoted 40 lines · Brian Foster

Page writeback indirectly handles shared extents via the existence of overlapping COW fork blocks. If COW fork blocks exist, writeback always performs the associated copy-on-write regardless if the underlying blocks are actually shared. If the blocks are shared, then overlapping COW fork blocks must always exist.

fstests shared/010 reproduces a case where a buffered write occurs over a shared block without performing the requisite COW fork reservation. This ultimately causes writeback to the shared extent and data corruption that is detected across md5 checks of the filesystem across a mount cycle.

The problem occurs when a buffered write lands over a shared extent that crosses an extent size hint boundary and that also happens to have a partial COW reservation that doesn't cover the start and end blocks of the data fork extent.

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Update xfs_reflink_reserve_cow() to accommodate the fact that a delalloc allocation request may not fully cover the extent in the data fork. Trim the data fork extent appropriately, just as is done for shared extent boundaries and/or existing COW reservations that happen to overlap the start of the data fork extent. This prevents shared/010 failures due to data corruption on reflink enabled filesystems.

Quoted 8 lines

Signed-off-by Brian Foster <[email protected]>

---

This is not fully tested yet beyond verification that it solves the problem reproduced by shared/010. I'll be running more tests today, but I'm sending sooner for review and testing due to the nature of the problem and the fact that it's a fairly isolated change. I'll follow up if I discover any resulting regressions..

Did you find any regressions?

Quoted 3 lines

I ran this through my overnight tests and saw no adverse effects, though Dave was complaining yesterday about continuing generic/091 corruptions (which I didn't see with this patch applied...)

I can say now that this patch hasn't caused any new corruptions. It hasn't fixed any of the (many) corruptions that I'm hitting, either, so from that perspective it's no better or worse than what we have now :P

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Quoted 57 lines · Dave Chinner

Quoted 55 lines · Darrick J. Wong

Quoted 40 lines · Brian Foster

Page writeback indirectly handles shared extents via the existence of overlapping COW fork blocks. If COW fork blocks exist, writeback always performs the associated copy-on-write regardless if the underlying blocks are actually shared. If the blocks are shared, then overlapping COW fork blocks must always exist.

fstests shared/010 reproduces a case where a buffered write occurs over a shared block without performing the requisite COW fork reservation. This ultimately causes writeback to the shared extent and data corruption that is detected across md5 checks of the filesystem across a mount cycle.

The problem occurs when a buffered write lands over a shared extent that crosses an extent size hint boundary and that also happens to have a partial COW reservation that doesn't cover the start and end blocks of the data fork extent.

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Update xfs_reflink_reserve_cow() to accommodate the fact that a delalloc allocation request may not fully cover the extent in the data fork. Trim the data fork extent appropriately, just as is done for shared extent boundaries and/or existing COW reservations that happen to overlap the start of the data fork extent. This prevents shared/010 failures due to data corruption on reflink enabled filesystems.

Quoted 8 lines

Signed-off-by Brian Foster <[email protected]>

---

This is not fully tested yet beyond verification that it solves the problem reproduced by shared/010. I'll be running more tests today, but I'm sending sooner for review and testing due to the nature of the problem and the fact that it's a fairly isolated change. I'll follow up if I discover any resulting regressions..

Did you find any regressions?

I ran this through my overnight tests and saw no adverse effects, though Dave was complaining yesterday about continuing generic/091 corruptions (which I didn't see with this patch applied...)

Quoted 4 lines

I can say now that this patch hasn't caused any new corruptions. It hasn't fixed any of the (many) corruptions that I'm hitting, either, so from that perspective it's no better or worse than what we have now :P

So were you reproducing the shared/010 corruption or no?

Quoted 63 lines · Brian Foster

Quoted 61 lines · Dave Chinner

Quoted 50 lines · Darrick J. Wong

Quoted 40 lines · Brian Foster

Page writeback indirectly handles shared extents via the existence of overlapping COW fork blocks. If COW fork blocks exist, writeback always performs the associated copy-on-write regardless if the underlying blocks are actually shared. If the blocks are shared, then overlapping COW fork blocks must always exist.

fstests shared/010 reproduces a case where a buffered write occurs over a shared block without performing the requisite COW fork reservation. This ultimately causes writeback to the shared extent and data corruption that is detected across md5 checks of the filesystem across a mount cycle.

The problem occurs when a buffered write lands over a shared extent that crosses an extent size hint boundary and that also happens to have a partial COW reservation that doesn't cover the start and end blocks of the data fork extent.

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Update xfs_reflink_reserve_cow() to accommodate the fact that a delalloc allocation request may not fully cover the extent in the data fork. Trim the data fork extent appropriately, just as is done for shared extent boundaries and/or existing COW reservations that happen to overlap the start of the data fork extent. This prevents shared/010 failures due to data corruption on reflink enabled filesystems.

Quoted 8 lines

Signed-off-by Brian Foster <[email protected]>

---

This is not fully tested yet beyond verification that it solves the problem reproduced by shared/010. I'll be running more tests today, but I'm sending sooner for review and testing due to the nature of the problem and the fact that it's a fairly isolated change. I'll follow up if I discover any resulting regressions..

Quoted 5 lines

Did you find any regressions?

I ran this through my overnight tests and saw no adverse effects, though Dave was complaining yesterday about continuing generic/091 corruptions (which I didn't see with this patch applied...)

I can say now that this patch hasn't caused any new corruptions. It hasn't fixed any of the (many) corruptions that I'm hitting, either, so from that perspective it's no better or worse than what we have now :P

Quoted 1 line

So were you reproducing the shared/010 corruption or no?

No, I wasn't, because I already had a patch in my tree that fixed it, apparently (see followup to Darrick's flush-after-zero on reflink patch). Basically, the EOF zeroing+truncation problem is something fsx trips over quite quickly on 1k block size filesystems. I've had a patch for it in my tree for about a week now.

So what I meant is that it didn't fix any of the fsx failures I'd been seeing, but it also didn't introduce any new fsx failures, either, as I was kind of hoping it would....

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Quoted 10 lines · Brian Foster

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

Quoted 4 lines

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Can you write this in the form of an xfstests reproducer please? :)

-Eric

Quoted 17 lines · Eric Sandeen

Quoted 14 lines · Brian Foster

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Quoted 2 lines

Can you write this in the form of an xfstests reproducer please? :)

I'll add it to the todo list.

Quoted 18 lines · Brian Foster

Quoted 17 lines · Eric Sandeen

Quoted 14 lines · Brian Foster

For example, a buffered write occurs across the file offset (in FSB units) range of [29, 57]. A shared extent exists at blocks [29, 35] and COW reservation already exists at blocks [32, 34]. After accommodating a COW extent size hint of 32 blocks and the existing reservation at offset 32, xfs_reflink_reserve_cow() allocates 32 blocks of reservation at offset 0 and returns with COW reservation across the range of [0, 34]. The associated data fork extent is still [29, 35], however, which isn't fully covered by the COW reservation.

This leads to a buffered write at file offset 35 over a shared extent without associated COW reservation. Writeback eventually kicks in, performs an overwrite of the underlying shared block and causes the associated data corruption.

Quoted 4 lines

Quoted 2 lines

Can you write this in the form of an xfstests reproducer please? :)

I'll add it to the todo list.

thanks, it doesn't seem like the kind of thing that will be hit too often at random, based on the struggles to reproduce it as first reported via fsstress.

-Eric