| Message ID | 20170308163336.12133-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Mar 08, 2017 at 08:33:36AM -0800, Christoph Hellwig wrote: > There are two different cases of buffered I/O errors: > > - first we can have an already shutdown fs. In that case we should skip > any on-disk operations and just clean up the appen transaction if > present and destroy the ioend > - a real I/O error. In that case we should cleanup any lingering COW > blocks. This gets skipped in the current code and is fixed by this > patch. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> (I pushed the 'reclaim unwritten COW extents periodically' patch and a fixed up version of this one out for testing last night. My fixed version matches this one exactly, so everything looks ok.) --D > > --- > > Note: "[PATCH v3] xfs: only reclaim unwritten COW extents periodically" > needs to be applied before this one. > > fs/xfs/xfs_aops.c | 59 +++++++++++++++++++++++++------------------------------ > 1 file changed, 27 insertions(+), 32 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index adea9da29c4b..eef453adbc06 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -274,54 +274,49 @@ xfs_end_io( > struct xfs_ioend *ioend = > container_of(work, struct xfs_ioend, io_work); > struct xfs_inode *ip = XFS_I(ioend->io_inode); > + xfs_off_t offset = ioend->io_offset; > + size_t size = ioend->io_size; > int error = ioend->io_bio->bi_error; > > /* > - * Set an error if the mount has shut down and proceed with end I/O > - * processing so it can perform whatever cleanups are necessary. > + * Just clean up the in-memory strutures if the fs has been shut down. > */ > - if (XFS_FORCED_SHUTDOWN(ip->i_mount)) > + if (XFS_FORCED_SHUTDOWN(ip->i_mount)) { > error = -EIO; > + goto done; > + } > > /* > - * For a CoW extent, we need to move the mapping from the CoW fork > - * to the data fork. If instead an error happened, just dump the > - * new blocks. > + * Clean up any COW blocks on an I/O error. > */ > - if (ioend->io_type == XFS_IO_COW) { > - if (error) > - goto done; > - if (ioend->io_bio->bi_error) { > - error = xfs_reflink_cancel_cow_range(ip, > - ioend->io_offset, ioend->io_size, true); > - goto done; > + if (unlikely(error)) { > + switch (ioend->io_type) { > + case XFS_IO_COW: > + xfs_reflink_cancel_cow_range(ip, offset, size, true); > + break; > } > - error = xfs_reflink_end_cow(ip, ioend->io_offset, > - ioend->io_size); > - if (error) > - goto done; > + > + goto done; > } > > /* > - * For unwritten extents we need to issue transactions to convert a > - * range to normal written extens after the data I/O has finished. > - * Detecting and handling completion IO errors is done individually > - * for each case as different cleanup operations need to be performed > - * on error. > + * Success: commit the COW or unwritten blocks if needed. > */ > - if (ioend->io_type == XFS_IO_UNWRITTEN) { > - if (error) > - goto done; > - error = xfs_iomap_write_unwritten(ip, ioend->io_offset, > - ioend->io_size); > - } else if (ioend->io_append_trans) { > - error = xfs_setfilesize_ioend(ioend, error); > - } else { > - ASSERT(!xfs_ioend_is_append(ioend) || > - ioend->io_type == XFS_IO_COW); > + switch (ioend->io_type) { > + case XFS_IO_COW: > + error = xfs_reflink_end_cow(ip, offset, size); > + break; > + case XFS_IO_UNWRITTEN: > + error = xfs_iomap_write_unwritten(ip, offset, size); > + break; > + default: > + ASSERT(!xfs_ioend_is_append(ioend) || ioend->io_append_trans); > + break; > } > > done: > + if (ioend->io_append_trans) > + error = xfs_setfilesize_ioend(ioend, error); > xfs_destroy_ioend(ioend, error); > } > > -- > 2.11.0 > > -- > 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 -- 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
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index adea9da29c4b..eef453adbc06 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -274,54 +274,49 @@ xfs_end_io( struct xfs_ioend *ioend = container_of(work, struct xfs_ioend, io_work); struct xfs_inode *ip = XFS_I(ioend->io_inode); + xfs_off_t offset = ioend->io_offset; + size_t size = ioend->io_size; int error = ioend->io_bio->bi_error; /* - * Set an error if the mount has shut down and proceed with end I/O - * processing so it can perform whatever cleanups are necessary. + * Just clean up the in-memory strutures if the fs has been shut down. */ - if (XFS_FORCED_SHUTDOWN(ip->i_mount)) + if (XFS_FORCED_SHUTDOWN(ip->i_mount)) { error = -EIO; + goto done; + } /* - * For a CoW extent, we need to move the mapping from the CoW fork - * to the data fork. If instead an error happened, just dump the - * new blocks. + * Clean up any COW blocks on an I/O error. */ - if (ioend->io_type == XFS_IO_COW) { - if (error) - goto done; - if (ioend->io_bio->bi_error) { - error = xfs_reflink_cancel_cow_range(ip, - ioend->io_offset, ioend->io_size, true); - goto done; + if (unlikely(error)) { + switch (ioend->io_type) { + case XFS_IO_COW: + xfs_reflink_cancel_cow_range(ip, offset, size, true); + break; } - error = xfs_reflink_end_cow(ip, ioend->io_offset, - ioend->io_size); - if (error) - goto done; + + goto done; } /* - * For unwritten extents we need to issue transactions to convert a - * range to normal written extens after the data I/O has finished. - * Detecting and handling completion IO errors is done individually - * for each case as different cleanup operations need to be performed - * on error. + * Success: commit the COW or unwritten blocks if needed. */ - if (ioend->io_type == XFS_IO_UNWRITTEN) { - if (error) - goto done; - error = xfs_iomap_write_unwritten(ip, ioend->io_offset, - ioend->io_size); - } else if (ioend->io_append_trans) { - error = xfs_setfilesize_ioend(ioend, error); - } else { - ASSERT(!xfs_ioend_is_append(ioend) || - ioend->io_type == XFS_IO_COW); + switch (ioend->io_type) { + case XFS_IO_COW: + error = xfs_reflink_end_cow(ip, offset, size); + break; + case XFS_IO_UNWRITTEN: + error = xfs_iomap_write_unwritten(ip, offset, size); + break; + default: + ASSERT(!xfs_ioend_is_append(ioend) || ioend->io_append_trans); + break; } done: + if (ioend->io_append_trans) + error = xfs_setfilesize_ioend(ioend, error); xfs_destroy_ioend(ioend, error); }
There are two different cases of buffered I/O errors: - first we can have an already shutdown fs. In that case we should skip any on-disk operations and just clean up the appen transaction if present and destroy the ioend - a real I/O error. In that case we should cleanup any lingering COW blocks. This gets skipped in the current code and is fixed by this patch. Signed-off-by: Christoph Hellwig <hch@lst.de> --- Note: "[PATCH v3] xfs: only reclaim unwritten COW extents periodically" needs to be applied before this one. fs/xfs/xfs_aops.c | 59 +++++++++++++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 32 deletions(-)