[PATCH] xfs: clear delalloc and cache on buffered write failure

[PATCH] xfs: clear delalloc and cache on buffered write failure

[Brian Foster]

commit fa7f138ac4c70dc00519c124cf7cd4862a0a5b0e upstream.

The buffered write failure handling code in
xfs_file_iomap_end_delalloc() has a couple minor problems. First, if
written == 0, start_fsb is not rounded down and it fails to kill off a
delalloc block if the start offset is block unaligned. This results in a
lingering delalloc block and broken delalloc block accounting detected
at unmount time. Fix this by rounding down start_fsb in the unlikely
event that written == 0.

Second, it is possible for a failed overwrite of a delalloc extent to
leave dirty pagecache around over a hole in the file. This is because is
possible to hit ->iomap_end() on write failure before the iomap code has
attempted to allocate pagecache, and thus has no need to clean it up. If
the targeted delalloc extent was successfully written by a previous
write, however, then it does still have dirty pages when ->iomap_end()
punches out the underlying blocks. This ultimately results in writeback
over a hole. To fix this problem, unconditionally punch out the
pagecache from XFS before the associated delalloc range.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
CC: stable@vger.kernel.org # v4.8+
---

This is for stable... it fixes a latent regression exposed by
d1908f52557b ("fs: break out of iomap_file_buffered_write on fatal
signals"), which is to be backported to 4.8+ stable kernels.

Brian

 fs/xfs/xfs_iomap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 2af0dda..fe3562b 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1064,7 +1064,15 @@ xfs_file_iomap_end_delalloc(
 	xfs_fileoff_t		end_fsb;
 	int			error = 0;
 
-	start_fsb = XFS_B_TO_FSB(mp, offset + written);
+	/*
+	 * start_fsb refers to the first unused block after a short write. If
+	 * nothing was written, round offset down to point at the first block in
+	 * the range.
+	 */
+	if (unlikely(!written))
+		start_fsb = XFS_B_TO_FSBT(mp, offset);
+	else
+		start_fsb = XFS_B_TO_FSB(mp, offset + written);
 	end_fsb = XFS_B_TO_FSB(mp, offset + length);
 
 	/*
@@ -1076,6 +1084,9 @@ xfs_file_iomap_end_delalloc(
 	 * blocks in the range, they are ours.
 	 */
 	if (start_fsb < end_fsb) {
+		truncate_pagecache_range(VFS_I(ip), XFS_FSB_TO_B(mp, start_fsb),
+					 XFS_FSB_TO_B(mp, end_fsb) - 1);
+
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
 		error = xfs_bmap_punch_delalloc_range(ip, start_fsb,
 					       end_fsb - start_fsb);
-- 
2.7.4

Re: [PATCH] xfs: clear delalloc and cache on buffered write failure

[Greg KH]

On Tue, Feb 21, 2017 at 08:46:27AM -0500, Brian Foster wrote:
> commit fa7f138ac4c70dc00519c124cf7cd4862a0a5b0e upstream.
> 
> The buffered write failure handling code in
> xfs_file_iomap_end_delalloc() has a couple minor problems. First, if
> written == 0, start_fsb is not rounded down and it fails to kill off a
> delalloc block if the start offset is block unaligned. This results in a
> lingering delalloc block and broken delalloc block accounting detected
> at unmount time. Fix this by rounding down start_fsb in the unlikely
> event that written == 0.
> 
> Second, it is possible for a failed overwrite of a delalloc extent to
> leave dirty pagecache around over a hole in the file. This is because is
> possible to hit ->iomap_end() on write failure before the iomap code has
> attempted to allocate pagecache, and thus has no need to clean it up. If
> the targeted delalloc extent was successfully written by a previous
> write, however, then it does still have dirty pages when ->iomap_end()
> punches out the underlying blocks. This ultimately results in writeback
> over a hole. To fix this problem, unconditionally punch out the
> pagecache from XFS before the associated delalloc range.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> CC: stable@vger.kernel.org # v4.8+
> ---
> 
> This is for stable... it fixes a latent regression exposed by
> d1908f52557b ("fs: break out of iomap_file_buffered_write on fatal
> signals"), which is to be backported to 4.8+ stable kernels.

For XFS patches, I need an explicit ack from the maintainer before
applying them, as per their instructions in the past...

thanks,

greg k-h

Re: [PATCH] xfs: clear delalloc and cache on buffered write failure

[Christoph Hellwig]

On Tue, Feb 21, 2017 at 02:56:39PM +0100, Greg KH wrote:
> For XFS patches, I need an explicit ack from the maintainer before
> applying them, as per their instructions in the past...

Brian is one of the most active XFS contributors, you can trust him :)

Re: [PATCH] xfs: clear delalloc and cache on buffered write failure

[Greg KH]

On Tue, Feb 21, 2017 at 06:40:00AM -0800, Christoph Hellwig wrote:
> On Tue, Feb 21, 2017 at 02:56:39PM +0100, Greg KH wrote:
> > For XFS patches, I need an explicit ack from the maintainer before
> > applying them, as per their instructions in the past...
> 
> Brian is one of the most active XFS contributors, you can trust him :)

Heh, ok, but I have been yelled at before for xfs stuff, so I'm a bit
skitish...

I'll queue this up for the next release, after this one that just
started a few hours ago.

thanks,

greg k-h

Re: [PATCH] xfs: clear delalloc and cache on buffered write failure

[Darrick J. Wong]

On Tue, Feb 21, 2017 at 02:56:39PM +0100, Greg KH wrote:
> On Tue, Feb 21, 2017 at 08:46:27AM -0500, Brian Foster wrote:
> > commit fa7f138ac4c70dc00519c124cf7cd4862a0a5b0e upstream.
> > 
> > The buffered write failure handling code in
> > xfs_file_iomap_end_delalloc() has a couple minor problems. First, if
> > written == 0, start_fsb is not rounded down and it fails to kill off a
> > delalloc block if the start offset is block unaligned. This results in a
> > lingering delalloc block and broken delalloc block accounting detected
> > at unmount time. Fix this by rounding down start_fsb in the unlikely
> > event that written == 0.
> > 
> > Second, it is possible for a failed overwrite of a delalloc extent to
> > leave dirty pagecache around over a hole in the file. This is because is
> > possible to hit ->iomap_end() on write failure before the iomap code has
> > attempted to allocate pagecache, and thus has no need to clean it up. If
> > the targeted delalloc extent was successfully written by a previous
> > write, however, then it does still have dirty pages when ->iomap_end()
> > punches out the underlying blocks. This ultimately results in writeback
> > over a hole. To fix this problem, unconditionally punch out the
> > pagecache from XFS before the associated delalloc range.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > CC: stable@vger.kernel.org # v4.8+
> > ---
> > 
> > This is for stable... it fixes a latent regression exposed by
> > d1908f52557b ("fs: break out of iomap_file_buffered_write on fatal
> > signals"), which is to be backported to 4.8+ stable kernels.
> 
> For XFS patches, I need an explicit ack from the maintainer before
> applying them, as per their instructions in the past...

"ACK", FWIW.

(XFS is slowly moving towards group maintainership amongst the most
active contributors, or at least "don't lets dump all the responsibility
on Dave" maintainership.)

--D

> 
> thanks,
> 
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html