Skip to contentSkip to conversation

xfs: fix error handling in xfs_refcount_insert()

PATCH In review Dave Chinner 2 participants 2 messages May 31, 2018 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs: fix error handling in xfs_refcount_insert()

From: Dave Chinner <[email protected]>

generic/475 fired an assert failure just after the filesystem was shut down:

XFS: Assertion failed: fs_is_ok, file: fs/xfs/libxfs/xfs_refcount.c, line: 182
.....

Call Trace:
 xfs_refcount_insert+0x151/0x190
 xfs_refcount_adjust_extents.constprop.11+0x9c/0x470
 xfs_refcount_adjust.constprop.10+0xb0/0x270
 xfs_refcount_finish_one+0x25a/0x420
 xfs_trans_log_finish_refcount_update+0x2a/0x40
 xfs_refcount_update_finish_item+0x35/0xa0
 xfs_defer_finish+0x15e/0x4d0
 xfs_reflink_remap_extent+0x1bc/0x610
 xfs_reflink_remap_blocks+0x6e/0x280
 xfs_reflink_remap_range+0x311/0x530
 vfs_clone_file_range+0x119/0x200
 ....

If xfs_btree_insert() returns an error, the corruption check fires instead of passing the error back the caller. The corruption check should be after we've checked for an error, not before, thereby avoiding assert failures if the filesystem shuts down during a refcount btree record insert.

---
 fs/xfs/libxfs/xfs_refcount.c | 3 +++
 1 file changed, 3 insertions(+)
fs/xfs/libxfs/xfs_refcount.c +3 -0
index ae07a85d9bd5..235a9cf7265f 100644
@@ -165,7 +165,10 @@ xfs_refcount_insert(
 	cur->bc_rec.rc.rc_blockcount = irec->rc_blockcount;
 	cur->bc_rec.rc.rc_refcount = irec->rc_refcount;
 	error = xfs_btree_insert(cur, i);
+	if (error)
+		goto out_error;
 	XFS_WANT_CORRUPTED_GOTO(cur->bc_mp, *i == 1, out_error);
+
 out_error:
 	if (error)
 		trace_xfs_refcount_insert_error(cur->bc_mp,
Signature · 1 line
2.17.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 21 lines · Dave Chinner

From: Dave Chinner <[email protected]>

generic/475 fired an assert failure just after the filesystem was shut down:

XFS: Assertion failed: fs_is_ok, file: fs/xfs/libxfs/xfs_refcount.c, line: 182
.....

Call Trace:
 xfs_refcount_insert+0x151/0x190
 xfs_refcount_adjust_extents.constprop.11+0x9c/0x470
 xfs_refcount_adjust.constprop.10+0xb0/0x270
 xfs_refcount_finish_one+0x25a/0x420
 xfs_trans_log_finish_refcount_update+0x2a/0x40
 xfs_refcount_update_finish_item+0x35/0xa0
 xfs_defer_finish+0x15e/0x4d0
 xfs_reflink_remap_extent+0x1bc/0x610
 xfs_reflink_remap_blocks+0x6e/0x280
 xfs_reflink_remap_range+0x311/0x530
 vfs_clone_file_range+0x119/0x200
 ....
Quoted 7 lines

If xfs_btree_insert() returns an error, the corruption check fires instead of passing the error back the caller. The corruption check should be after we've checked for an error, not before, thereby avoiding assert failures if the filesystem shuts down during a refcount btree record insert.

HAAA I ARE DUMMM

--D

Quoted 26 lines
---
 fs/xfs/libxfs/xfs_refcount.c | 3 +++
 1 file changed, 3 insertions(+)
fs/xfs/libxfs/xfs_refcount.c +3 -0
index ae07a85d9bd5..235a9cf7265f 100644
@@ -165,7 +165,10 @@ xfs_refcount_insert(
 	cur->bc_rec.rc.rc_blockcount = irec->rc_blockcount;
 	cur->bc_rec.rc.rc_refcount = irec->rc_refcount;
 	error = xfs_btree_insert(cur, i);
+	if (error)
+		goto out_error;
 	XFS_WANT_CORRUPTED_GOTO(cur->bc_mp, *i == 1, out_error);
+
 out_error:
 	if (error)
 		trace_xfs_refcount_insert_error(cur->bc_mp,
Signature · 6 lines
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html