On 10/23/18 6:30 PM, Qu Wenruo wrote:
On 2018/10/23 下午5:41, Su Yue wrote:
Previously, @err are assigned immediately after check but before
repair.
repair_extent_item()'s return value also confuses the caller. If
error has been repaired and returns 0, check_extent_item() will try
to continue check the nonexistent and cause flase alerts.
Here make repair_extent_item()'s return codes only represents status
of the extent item, error bits are passed by pointer.
Move the change of @err after repair.
Signed-off-by: Su Yue <suy.fnst@xxxxxxxxxxxxxx>
---
check/mode-lowmem.c | 106 ++++++++++++++++++++++++++++----------------
1 file changed, 68 insertions(+), 38 deletions(-)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 76e7be81ceb1..769b3141de8b 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -3788,35 +3788,35 @@ out:
/*
* Only delete backref if REFERENCER_MISSING now
*
- * Returns <0 the extent was deleted
- * Returns >0 the backref was deleted but extent still exists, returned value
- * means error after repair
+ * Returns <0 the whole extent item was deleted
+ * Returns >0 the backref was deleted but extent still exists, @err_ret
+ * will be set to be error bits after repair.
Well, I have to admit the parameter list is really human friendly from
the initial design.
As my previous design, callers of repair_xxx functions just take
care of successes, failures and pathes. If exposing the logic
to the callers is acceptable, will do it as your suggestions.
Thanks,
Su
In fact, the only real user of @err is just that "if ((err &
(REFERENCER_MISSING | REFERENCER_MISMATCH)) == 0)" line.
We can in fact get rid of that @err parameter and do that check at the
only caller of repair_extent_item().
And then redefine the return value.
Like "Return < 0 if we failed to repair backref for the extent.
Return 0 if we repaired the backref for the extent".
[snip]
- if (err && repair) {
+ if (tmp_err && repair) {
ret = repair_extent_item(fs_info->extent_root, path,
key.objectid, num_bytes, parent, root_objectid,
- owner, owner_offset, ret);
- if (ret < 0)
If follow my above method, here the code can be a little easier:
if (tmp_err && repair) {
if (!(tmp_err & (REFERENCER_MISSING |
REFENCER_MISMATCH)) {
err |= tmp_err;
goto next;
}
ret = repair_extent_item(...);
if (ret < 0)
err |= tmp_err;
} else if (tmp_err) {
err |= tmp_err;
}
Without the need to passing @err into repair_extent_item() nor modifying
@err in that function.
Thanks,
Qu
+ owner, owner_offset, &tmp_err);
+ err |= tmp_err;
+
+ if (tmp_err & FATAL_ERROR || ret < 0)
goto out;
- if (ret) {
+ /*
+ * the error has been repaired which means the extent item
+ * is still existed with other backrefs, go to check next.
+ */> + if (ret > 0) {
+ eb = path->nodes[0];
+ slot = path->slots[0];
+ ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
+ item_size = btrfs_item_size_nr(eb, slot);
goto next;
- err = ret;
}
+ } else {
+ err |= tmp_err;
}
- ptr += btrfs_extent_inline_ref_size(type);
+ ptr_offset += btrfs_extent_inline_ref_size(type);
goto next;
out:
