From: Nikolay Borisov <nborisov@suse.com>
To: Su Yue <suy.fnst@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v3 6/7] btrfs-progs: lowmem: optimization and repair for check_inode_extref()
Date: Mon, 17 Sep 2018 18:25:07 +0300 [thread overview]
Message-ID: <d9b3fab2-4e5c-ffe8-77e8-d024c4e414aa@suse.com> (raw)
In-Reply-To: <20180917072852.25831-7-suy.fnst@cn.fujitsu.com>
On 17.09.2018 10:28, Su Yue wrote:
> inode_extref is much similar with inode_ref, most codes are reused in
> check_inode_extref().
> Exceptions:
> There is no need to check root directory, so remove it.
> Make check_inode_extref() verify hash value with key offset now.
>
> And lowmem check can detect errors about inode_extref and try to
> repair errors.
>
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
> check/mode-lowmem.c | 125 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 99 insertions(+), 26 deletions(-)
>
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 612e5e28e45b..53e4fdccd740 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1182,37 +1182,86 @@ out:
> *
> * Return 0 if no error occurred.
> */
> -static int check_inode_extref(struct btrfs_root *root,
> - struct btrfs_key *ref_key,
> - struct extent_buffer *node, int slot, u64 *refs,
> - int mode)
> +static int check_inode_extref(struct btrfs_root *root, struct btrfs_key *ref_key,
> + struct btrfs_path *path, char *name_ret,
> + u32 *namelen_ret, u64 *refs_ret, int mode)
> {
> struct btrfs_key key;
> struct btrfs_key location;
> struct btrfs_inode_extref *extref;
> + struct extent_buffer *node;
> char namebuf[BTRFS_NAME_LEN] = {0};
> u32 total;
> - u32 cur = 0;
> + u32 cur;
> u32 len;
> u32 name_len;
> u64 index;
> u64 parent;
> + int err;
> + int tmp_err;
> int ret;
> - int err = 0;
> + int slot;
> + u64 refs;
> + bool search_again = false;
>
> location.objectid = ref_key->objectid;
> location.type = BTRFS_INODE_ITEM_KEY;
> location.offset = 0;
> +begin:
Please don't implement loops with labels, instead use the appropriate
loop construct. Otherwise just factor out the repetitive code in a
function and call that in a loop. Labels should ideally be used only for
error cases to terminate the current function.
> + cur = 0;
> + err = 0;
> + refs = *refs_ret;
> + *namelen_ret = 0;
> +
> + /* since after repair, path and the dir item may be changed */
> + if (search_again) {
> + search_again = false;
> + btrfs_release_path(path);
> + ret = btrfs_search_slot(NULL, root, ref_key, path, 0, 0);
> + /*
> + * The item was deleted, let the path point to the last checked
> + * item.
> + */
> + if (ret > 0) {
> + if (path->slots[0] == 0) {
> + ret = btrfs_prev_leaf(root, path);
> + /*
> + * we are checking the inode item, there must
> + * be some items before, the case shall never
> + * happen.
> + */
> + if (ret > 0)
> + ret = -EIO;
> + if (ret)
> + error(
> + "failed to get item before INODE_REF[%llu, %llu] root %llu: %s",
> + ref_key->objectid, ref_key->offset,
> + root->objectid, strerror(-ret));
> + } else {
> + path->slots[0]--;
> + ret = 0;
> + }
> + goto out;
> + } else if (ret < 0) {
> + err |= FATAL_ERROR;
> + goto out;
> + }
> + }
> +
> + node = path->nodes[0];
> + slot = path->slots[0];
>
> extref = btrfs_item_ptr(node, slot, struct btrfs_inode_extref);
> total = btrfs_item_size_nr(node, slot);
>
> -next:
> - /* update inode ref count */
> - (*refs)++;
> - name_len = btrfs_inode_extref_name_len(node, extref);
> - index = btrfs_inode_extref_index(node, extref);
> +loop:
> + /* Update inode ref count */
> + refs++;
> + tmp_err = 0;
> parent = btrfs_inode_extref_parent(node, extref);
> + index = btrfs_inode_extref_index(node, extref);
> + name_len = btrfs_inode_extref_name_len(node, extref);
> +
> if (name_len <= BTRFS_NAME_LEN) {
> len = name_len;
> } else {
> @@ -1220,37 +1269,61 @@ next:
> warning("root %llu INODE_EXTREF[%llu %llu] name too long",
> root->objectid, ref_key->objectid, ref_key->offset);
> }
> +
> read_extent_buffer(node, namebuf, (unsigned long)(extref + 1), len);
>
> - /* Check root dir ref name */
> - if (index == 0 && strncmp(namebuf, "..", name_len)) {
> - error("root %llu INODE_EXTREF[%llu %llu] ROOT_DIR name shouldn't be %s",
> + /* verify hash value */
> + if (ref_key->offset != btrfs_extref_hash(parent, namebuf, len)) {
> + err |= FATAL_ERROR;
> + error("root %llu INODE_EXTREF[%llu %llu] name %s namelen %u mode %d mismatch with its hash, wanted %llu have %llu",
> root->objectid, ref_key->objectid, ref_key->offset,
> - namebuf);
> - err |= ROOT_DIR_ERROR;
> + namebuf, len, mode, ref_key->offset,
> + btrfs_extref_hash(parent, namebuf, len));
> + goto out;
> + }
> +
> + /* copy the first name found to name_ret */
> + if (refs == 1 && name_ret) {
> + memcpy(name_ret, namebuf, len);
> + *namelen_ret = len;
> }
>
> - /* find related dir_index */
> + /* Find related DIR_INDEX */
> key.objectid = parent;
> key.type = BTRFS_DIR_INDEX_KEY;
> key.offset = index;
> - ret = find_dir_item(root, &key, &location, namebuf, len, mode);
> - err |= ret;
> + tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
> + imode_to_type(mode));
>
> - /* find related dir_item */
> + /* Find related dir_item */
> key.objectid = parent;
> key.type = BTRFS_DIR_ITEM_KEY;
> key.offset = btrfs_name_hash(namebuf, len);
> - ret = find_dir_item(root, &key, &location, namebuf, len, mode);
> - err |= ret;
> + tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
> + imode_to_type(mode));
> +
> + if (tmp_err && repair) {
> + ret = repair_ternary_lowmem(root, parent, ref_key->objectid,
> + index, namebuf, name_len, imode_to_type(mode),
> + tmp_err);
> + if (!ret) {
> + search_again = true;
> + goto begin;
> + }
> + }
>
> + print_inode_ref_err(root, ref_key, index, namebuf, name_len,
> + imode_to_type(mode), tmp_err);
> +
> + err |= tmp_err;
> len = sizeof(*extref) + name_len;
> extref = (struct btrfs_inode_extref *)((char *)extref + len);
> cur += len;
>
> if (cur < total)
> - goto next;
> -
> + goto loop;
> +out:
> + *refs_ret = refs;
> return err;
> }
>
> @@ -2426,8 +2499,8 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
> warning("root %llu EXTREF[%llu %llu] isn't supported",
> root->objectid, key.objectid,
> key.offset);
> - ret = check_inode_extref(root, &key, node, slot, &refs,
> - mode);
> + ret = check_inode_extref(root, &key, path, namebuf,
> + &name_len, &refs, mode);
> err |= ret;
> break;
> }
>
next prev parent reply other threads:[~2018-09-17 20:53 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-17 7:28 [PATCH v3 0/7] btrfs-progs: lowmem: bug fixes and inode_extref repair Su Yue
2018-09-17 7:28 ` [PATCH v3 1/7] btrfs-progs: adjust arguments of btrfs_lookup_inode_extref() Su Yue
2018-09-17 7:28 ` [PATCH v3 2/7] btrfs-progs: make btrfs_unlink() lookup inode_extref Su Yue
2018-09-17 7:28 ` [PATCH v3 3/7] btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item() Su Yue
2018-09-17 12:47 ` Qu Wenruo
2018-09-17 7:28 ` [PATCH v3 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() after repair Su Yue
2018-09-17 12:51 ` Qu Wenruo
2018-09-17 12:55 ` Su Yue
2018-09-17 13:03 ` Qu Wenruo
2018-09-17 13:13 ` Nikolay Borisov
2018-09-17 7:28 ` [PATCH v3 5/7] btrfs-progs: lowmem: do missing check of last item after check_inode_item() Su Yue
2018-09-17 12:53 ` Qu Wenruo
2018-09-17 13:24 ` Su Yue
2018-09-18 5:32 ` Qu Wenruo
2018-09-18 8:01 ` Su Yue
2018-09-18 8:15 ` Qu Wenruo
2018-09-17 7:28 ` [PATCH v3 6/7] btrfs-progs: lowmem: optimization and repair for check_inode_extref() Su Yue
2018-09-17 13:00 ` Qu Wenruo
2018-09-17 15:25 ` Nikolay Borisov [this message]
2018-09-17 7:28 ` [PATCH v3 7/7] btrfs-progs: fsck-tests: add test case inode_extref without dir_item and dir_index Su Yue
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d9b3fab2-4e5c-ffe8-77e8-d024c4e414aa@suse.com \
--to=nborisov@suse.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=suy.fnst@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).