cancel the setfilesize transation when io error happen
PATCH 1/1 cancel the setfilesize transation when io error happen
When I ran xfstest/073 case, the remount process was blocked to wait transactions to be zero. I found there was a io error happened, and the setfilesize transaction was not released properly. We should add the changes to cancel the io error in this case.
Reproduction steps:
1. dd if=/dev/zero of=xfs1.img bs=1M count=2048
2. mkfs.xfs xfs1.img
3. losetup -f ./xfs1.img /dev/loop0
4. mount -t xfs /dev/loop0 /home/test_dir/
5. mkdir /home/test_dir/test
6. mkfs.xfs -dfile,name=image,size=2g
7. mount -t xfs -o loop image /home/test_dir/test
8. cp a file bigger than 2g to /home/test_dir/test
9. mount -t xfs -o remount,ro /home/test_dir/test
--- fs/xfs/xfs_aops.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
fs/xfs/xfs_aops.c +14 -1
@@ -212,8 +212,21 @@ xfs_end_io( ioend->io_error = -EIO ; goto done; } -if (ioend->io_error) +if (ioend->io_error) { +/* +* We should cancel the setfilesize transation when io error +* happen. +*/ +if (ioend->io_append_trans) { +current_set_flags_nested (&ioend->io_append_trans->t_pflags, +PF_FSTRANS ); +rwsem_acquire_read (&VFS_I (XFS_I (ioend->io_inode))->i_sb + ->s_writers.lock_map[SB_FREEZE_FS -1 ],0 ,1 , _THIS_IP_); +xfs_trans_cancel (ioend->io_append_trans); + } + goto done; + }/* * For unwritten extents we need to issue transactions to convert a
Signature · 9 lines
1.8.3.1 -- version="9" logging="no" _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 5 lines · Zhaohongjiang
When I ran xfstest/073 case, the remount process was blocked to wait transactions to be zero. I found there was a io error happened, and the setfilesize transaction was not released properly. We should add the changes to cancel the io error in this case.
Thanks for the patch. Was the io error by chance or part of the test? I ask because it would be good to have test coverage for this if we don't currently.
Quoted 26 lines
Reproduction steps:
1. dd if=/dev/zero of=xfs1.img bs=1M count=2048
2. mkfs.xfs xfs1.img
3. losetup -f ./xfs1.img/dev/loop0
4. mount -t xfs/dev/loop0/home/test_dir/
5. mkdir/home/test_dir/test
6. mkfs.xfs -dfile,name=image,size=2g
7. mount -t xfs -o loop image/home/test_dir/test
8. cp a file bigger than 2g to/home/test_dir/test
9. mount -t xfs -o remount,ro/home/test_dir/testSigned-off-by Zhao Hongjiang <[email protected]>--- fs/xfs/xfs_aops.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +1
-1@@ -212,8 +212,21 @@ xfs_end_io( ioend->io_error = -EIO ; goto done; } -if (ioend->io_error) +if (ioend->io_error) {
fs/xfs/xfs_aops.c xfs_end_io( · line 225
Quoted 10 lines
fs/xfs/xfs_aops.c +10
-0+/* +* We should cancel the setfilesize transation when io error +* happen. +*/ +if (ioend->io_append_trans) { +current_set_flags_nested (&ioend->io_append_trans->t_pflags, +PF_FSTRANS ); +rwsem_acquire_read (&VFS_I (XFS_I (ioend->io_inode))->i_sb + ->s_writers.lock_map[SB_FREEZE_FS -1 ],0 ,1 , _THIS_IP_); +xfs_trans_cancel (ioend->io_append_trans);
Trailing whitespace on the line above.
That aside, I wonder if it would be cleaner to put this stuff in a helper similar to xfs_setfilesize_ioend(). In fact, we could probably just update xfs_setfilesize_ioend() to check ioend->io_error after fixing up the task flags and whatnot and cancel the transaction instead of calling xfs_setfilesize(). Then, call it here also and update the new comment to point out that it cancels the transaction on error. Thoughts?
Brian
Quoted 17 lines
fs/xfs/xfs_aops.c +3
-0+ } + goto done; + }/* * For unwritten extents we need to issue transactions to convert a Signature · 9 lines
1.8.3.1 -- version="9" logging="no" _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 50 lines · Brian Foster
Quoted 5 lines · Zhaohongjiang
When I ran xfstest/073 case, the remount process was blocked to wait transactions to be zero. I found there was a io error happened, and the setfilesize transaction was not released properly. We should add the changes to cancel the io error in this case.
Thanks for the patch. Was the io error by chance or part of the test? I ask because it would be good to have test coverage for this if we don't currently.
Quoted 26 lines
Reproduction steps:
1. dd if=/dev/zero of=xfs1.img bs=1M count=2048
2. mkfs.xfs xfs1.img
3. losetup -f ./xfs1.img/dev/loop0
4. mount -t xfs/dev/loop0/home/test_dir/
5. mkdir/home/test_dir/test
6. mkfs.xfs -dfile,name=image,size=2g
7. mount -t xfs -o loop image/home/test_dir/test
8. cp a file bigger than 2g to/home/test_dir/test
9. mount -t xfs -o remount,ro/home/test_dir/testSigned-off-by Zhao Hongjiang <[email protected]>--- fs/xfs/xfs_aops.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)fs/xfs/xfs_aops.c +1
-1@@ -212,8 +212,21 @@ xfs_end_io( ioend->io_error = -EIO ; goto done; } -if (ioend->io_error) +if (ioend->io_error) {fs/xfs/xfs_aops.c xfs_end_io( · line 225
Quoted 10 lines
fs/xfs/xfs_aops.c +10
-0+/* +* We should cancel the setfilesize transation when io error +* happen. +*/ +if (ioend->io_append_trans) { +current_set_flags_nested (&ioend->io_append_trans->t_pflags, +PF_FSTRANS ); +rwsem_acquire_read (&VFS_I (XFS_I (ioend->io_inode))->i_sb + ->s_writers.lock_map[SB_FREEZE_FS -1 ],0 ,1 , _THIS_IP_); +xfs_trans_cancel (ioend->io_append_trans);Trailing whitespace on the line above.
Quoted 6 lines
That aside, I wonder if it would be cleaner to put this stuff in a helper similar to
xfs_setfilesize_ioend(). In fact, we could probably just updatexfs_setfilesize_ioend()to checkioend->io_errorafter fixing up the task flags and whatnot and cancel the transaction instead of callingxfs_setfilesize(). Then, call it here also and update the new comment to point out that it cancels the transaction on error. Thoughts?
Agreed, I made this modification on commit. See below.
Cheers,
Dave.
Signature · 31 lines
Dave Chinner [email protected] cancel the setfilesize transation when io error happen From: Zhaohongjiang <[email protected]> When I ran xfstest/073 case, the remount process was blocked to wait transactions to be zero. I found there was a io error happened, and the setfilesize transaction was not released properly. We should add the changes to cancel the io error in this case. Reproduction steps: 1. dd if=/dev/zero of=xfs1.img bs=1M count=2048 2. mkfs.xfs xfs1.img 3. losetup -f ./xfs1.img /dev/loop0 4. mount -t xfs /dev/loop0 /home/test_dir/ 5. mkdir /home/test_dir/test 6. mkfs.xfs -dfile,name=image,size=2g 7. mount -t xfs -o loop image /home/test_dir/test 8. cp a file bigger than 2g to /home/test_dir/test 9. mount -t xfs -o remount,ro /home/test_dir/test [ dchinner: moved io error detection to xfs_setfilesize_ioend() after transaction context restoration. ] Signed-off-by: Zhao Hongjiang <[email protected]> Signed-off-by: Dave Chinner <[email protected]> --- fs/xfs/xfs_aops.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
fs/xfs/xfs_aops.c +11 -2
@@ -172,6 +172,12 @@ xfs_setfilesize_ioend(current_set_flags_nested (&tp->t_pflags,PF_FSTRANS );__sb_writers_acquired (VFS_I (ip)->i_sb,SB_FREEZE_FS ); +/* we abort the update if there was an IO error */ +if (ioend->io_error) { +xfs_trans_cancel (tp); +return ioend->io_error; + } +return xfs_setfilesize (ip, tp, ioend->io_offset, ioend->io_size); }
@@ -212,14 +218,17 @@ xfs_end_io( ioend->io_error = -EIO ; goto done; } -if (ioend->io_error) - 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. */ if (ioend->io_type ==XFS_IO_UNWRITTEN ) { +if (ioend->io_error) + goto done; error =xfs_iomap_write_unwritten (ip, ioend->io_offset, ioend->io_size); }else if (ioend->io_append_trans) {
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs