* [PATCH] xfs: fix and streamline error handling in xfs_end_io
@ 2017-03-08 16:33 Christoph Hellwig
2017-03-08 17:56 ` Darrick J. Wong
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2017-03-08 16:33 UTC (permalink / raw)
To: linux-xfs
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(-)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix and streamline error handling in xfs_end_io
2017-03-08 16:33 [PATCH] xfs: fix and streamline error handling in xfs_end_io Christoph Hellwig
@ 2017-03-08 17:56 ` Darrick J. Wong
0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2017-03-08 17:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix and streamline error handling in xfs_end_io
2017-02-28 0:49 Christoph Hellwig
@ 2017-02-28 5:34 ` Darrick J. Wong
0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2017-02-28 5:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Feb 27, 2017 at 04:49:50PM -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.
Somehow I lost that through all the rebasing of the reflink patchset,
thank you for catching this!
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> 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 290c0fb306e7..fe244648fff0 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, 0);
> - goto done;
> + if (unlikely(error)) {
> + switch (ioend->io_type) {
> + case XFS_IO_COW:
> + xfs_reflink_cancel_cow_range(ip, offset, size, 0);
> + 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] xfs: fix and streamline error handling in xfs_end_io
@ 2017-02-28 0:49 Christoph Hellwig
2017-02-28 5:34 ` Darrick J. Wong
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2017-02-28 0:49 UTC (permalink / raw)
To: linux-xfs
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>
---
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 290c0fb306e7..fe244648fff0 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, 0);
- goto done;
+ if (unlikely(error)) {
+ switch (ioend->io_type) {
+ case XFS_IO_COW:
+ xfs_reflink_cancel_cow_range(ip, offset, size, 0);
+ 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-08 17:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-08 16:33 [PATCH] xfs: fix and streamline error handling in xfs_end_io Christoph Hellwig
2017-03-08 17:56 ` Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2017-02-28 0:49 Christoph Hellwig
2017-02-28 5:34 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).