* [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes fix in lowmem
@ 2017-01-09 5:38 Su Yue
2017-01-09 5:38 ` [PATCH 2/2] btrfs-progs: cmds-check.c: supports inode isize " Su Yue
2017-01-17 15:40 ` [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes " David Sterba
0 siblings, 2 replies; 5+ messages in thread
From: Su Yue @ 2017-01-09 5:38 UTC (permalink / raw)
To: linux-btrfs
Added 'repair_inode_item' which dispatches functions such as
'repair_inode__nbytes_lowmem' to correct errors and
'struct inode_item_fix_info' to store correct values and errors.
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
cmds-check.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 152 insertions(+), 9 deletions(-)
diff --git a/cmds-check.c b/cmds-check.c
index 1dba298..567ca80 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -371,6 +371,17 @@ struct root_item_info {
};
/*
+ * Use inode_item_fix_info as function check_inode_item's arg.
+ */
+struct inode_item_fix_info {
+ u64 ino;
+ u64 isize;
+ u64 nbytes;
+
+ int err;
+};
+
+/*
* Error bit for low memory mode check.
*
* Currently no caller cares about it yet. Just internal use for error
@@ -1866,13 +1877,16 @@ struct node_refs {
static int update_nodes_refs(struct btrfs_root *root, u64 bytenr,
struct node_refs *nrefs, u64 level);
static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
- unsigned int ext_ref);
-
+ unsigned int ext_ref,
+ struct inode_item_fix_info *info);
+static int repair_inode_item(struct btrfs_root *root,
+ struct inode_item_fix_info *info);
static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
struct node_refs *nrefs, int *level, int ext_ref)
{
struct extent_buffer *cur = path->nodes[0];
struct btrfs_key key;
+ struct inode_item_fix_info info;
u64 cur_bytenr;
u32 nritems;
u64 first_ino = 0;
@@ -1881,6 +1895,7 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
int ret = 0; /* Final return value */
int err = 0; /* Positive error bitmap */
+ memset(&info, 0, sizeof(info));
cur_bytenr = cur->start;
/* skip to first inode item or the first inode number change */
@@ -1900,8 +1915,26 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
path->slots[0] = i;
again:
- err |= check_inode_item(root, path, ext_ref);
+ err |= check_inode_item(root, path, ext_ref, &info);
+
+ if (repair && (err & ~LAST_ITEM)) {
+ ret = repair_inode_item(root, &info);
+ if (ret < 0)
+ goto out;
+ /*
+ * if some errors was repaired, path shall be searched
+ * again since path has been changed
+ */
+ if (ret == 0) {
+ btrfs_item_key_to_cpu(path->nodes[0], &key,
+ path->slots[0]);
+ btrfs_release_path(path);
+ btrfs_search_slot(NULL, root, &key, path, 0, 0);
+
+ cur = path->nodes[0];
+ }
+ }
if (err & LAST_ITEM)
goto out;
@@ -2211,7 +2244,8 @@ out:
}
static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
- unsigned int ext_ref);
+ unsigned int ext_ref,
+ struct inode_item_fix_info *info);
static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
int *level, struct node_refs *nrefs, int ext_ref)
@@ -2293,7 +2327,7 @@ static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
}
ret = check_child_node(root, cur, path->slots[*level], next);
- if (ret < 0)
+ if (ret < 0)
break;
if (btrfs_is_leaf(next))
@@ -2383,6 +2417,105 @@ out:
return ret;
}
+/*
+ * Set inode's nbytes to correct value in @info
+ *
+ * Returns <0 means on error
+ * Returns 0 means successful repair
+ */
+static int repair_inode_nbytes_lowmem(struct btrfs_trans_handle *trans,
+ struct btrfs_root *root,
+ struct inode_item_fix_info *info)
+{
+ struct btrfs_inode_item *ei;
+ struct btrfs_key key;
+ struct btrfs_path path;
+ int ret;
+
+ ASSERT(info);
+ key.objectid = info->ino;
+ key.type = BTRFS_INODE_ITEM_KEY;
+ key.offset = 0;
+
+ ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
+ if (ret < 0)
+ goto out;
+ if (ret > 0) {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ ei = btrfs_item_ptr(path.nodes[0], path.slots[0],
+ struct btrfs_inode_item);
+ btrfs_set_inode_nbytes(path.nodes[0], ei, info->nbytes);
+ btrfs_mark_buffer_dirty(path.nodes[0]);
+ printf("reset nbytes for inode %llu root %llu\n", info->ino,
+ root->root_key.objectid);
+out:
+ btrfs_release_path(&path);
+ return ret;
+}
+
+/*
+ * repair_inode_item - repair inode item errors
+ *
+ * Repair the inode item if error can be repaired. Any caller should compare
+ * @info->err with the before, if err diffs, some contexts should be notified.
+ *
+ * @root: subvolume_root
+ * @info: contains correct values and error
+ * when it returns, @info stores the unrepairable errors
+ *
+ * Returns < 0 represents repair function error
+ * Returns 0 represents have fixed some errors or no error at all
+ * Returns > 0 represents it can't fix any error
+ */
+static int repair_inode_item(struct btrfs_root *root,
+ struct inode_item_fix_info *info)
+{
+ struct btrfs_trans_handle *trans;
+ int ret = 0;
+ int err;
+
+ ASSERT(info);
+ err = info->err;
+ if (!err) {
+ /* nothing to repair */
+ ret = 0;
+ goto out;
+ }
+ if (!(err & NBYTES_ERROR)) {
+ warning("root %llu INODE[%llu] have error(s) can't repair, error : %d",
+ root->objectid, info->ino, err);
+ /* can't fix any errors, ret should be positive */
+ ret = err;
+ goto out;
+ }
+
+ trans = btrfs_start_transaction(root, 1);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ goto out;
+ }
+
+ if (err & NBYTES_ERROR) {
+ ret = repair_inode_nbytes_lowmem(trans, root, info);
+ if (ret == 0)
+ err &= ~NBYTES_ERROR;
+ else if (ret < 0)
+ goto out;
+ }
+
+ if (err != info->err) {
+ info->err = err;
+ ret = 0;
+ }
+
+ btrfs_commit_transaction(trans, root);
+out:
+ return ret;
+}
+
static int repair_inode_isize(struct btrfs_trans_handle *trans,
struct btrfs_root *root, struct btrfs_path *path,
struct inode_record *rec)
@@ -4781,7 +4914,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
* Return >0 for error or hit the traversal is done(by error bitmap)
*/
static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
- unsigned int ext_ref)
+ unsigned int ext_ref,
+ struct inode_item_fix_info *info)
{
struct extent_buffer *node;
struct btrfs_inode_item *ii;
@@ -4821,6 +4955,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
dir = imode_to_type(mode) == BTRFS_FT_DIR;
nlink = btrfs_inode_nlink(node, ii);
nodatasum = btrfs_inode_flags(node, ii) & BTRFS_INODE_NODATASUM;
+ info->ino = inode_id;
while (1) {
ret = btrfs_next_item(root, path);
@@ -4924,11 +5059,13 @@ out:
if (nbytes != extent_size) {
err |= NBYTES_ERROR;
+ info->nbytes = extent_size;
error("root %llu INODE[%llu] nbytes(%llu) not equal to extent_size(%llu)",
root->objectid, inode_id, nbytes, extent_size);
}
}
+ info->err = err & ~LAST_ITEM;
return err;
}
@@ -4936,9 +5073,11 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
{
struct btrfs_path path;
struct btrfs_key key;
+ struct inode_item_fix_info info;
int err = 0;
int ret;
+ memset(&info, 0, sizeof(info));
btrfs_init_path(&path);
key.objectid = BTRFS_FIRST_FREE_OBJECTID;
key.type = BTRFS_INODE_ITEM_KEY;
@@ -4952,12 +5091,17 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
err |= INODE_ITEM_MISSING;
}
- err |= check_inode_item(root, &path, ext_ref);
+ err |= check_inode_item(root, &path, ext_ref, &info);
err &= ~LAST_ITEM;
if (err && !ret)
ret = -EIO;
out:
btrfs_release_path(&path);
+ if (repair) {
+ ret = repair_inode_item(root, &info);
+ if (!info.err)
+ ret = 0;
+ }
return ret;
}
@@ -12722,8 +12866,7 @@ int cmd_check(int argc, char **argv)
* Not supported yet
*/
if (repair && check_mode == CHECK_MODE_LOWMEM) {
- error("low memory mode doesn't support repair yet");
- exit(1);
+ error("low memory mode supports repair partailly");
}
radix_tree_init();
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] btrfs-progs: cmds-check.c: supports inode isize fix in lowmem
2017-01-09 5:38 [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes fix in lowmem Su Yue
@ 2017-01-09 5:38 ` Su Yue
2017-01-17 15:46 ` David Sterba
2017-01-17 15:40 ` [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes " David Sterba
1 sibling, 1 reply; 5+ messages in thread
From: Su Yue @ 2017-01-09 5:38 UTC (permalink / raw)
To: linux-btrfs
Add a function 'repair_inode_isize' to support inode isize repair.
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
cmds-check.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/cmds-check.c b/cmds-check.c
index 567ca80..088c0d8 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -2457,6 +2457,45 @@ out:
}
/*
+ * Set inode's isize to correct value in @info
+ *
+ * Returns <0 means on error
+ * Returns 0 means successful repair
+ */
+static int repair_inode_isize_lowmem(struct btrfs_trans_handle *trans,
+ struct btrfs_root *root,
+ struct inode_item_fix_info *info)
+{
+ struct btrfs_inode_item *ei;
+ struct btrfs_key key;
+ struct btrfs_path path;
+ int ret;
+
+ ASSERT(info);
+ key.objectid = info->ino;
+ key.type = BTRFS_INODE_ITEM_KEY;
+ key.offset = 0;
+
+ ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
+ if (ret < 0)
+ goto out;
+ if (ret > 0) {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ ei = btrfs_item_ptr(path.nodes[0], path.slots[0],
+ struct btrfs_inode_item);
+ btrfs_set_inode_size(path.nodes[0], ei, info->isize);
+ btrfs_mark_buffer_dirty(path.nodes[0]);
+ printf("reset isize for inode %llu root %llu\n", info->ino,
+ root->root_key.objectid);
+out:
+ btrfs_release_path(&path);
+ return ret;
+}
+
+/*
* repair_inode_item - repair inode item errors
*
* Repair the inode item if error can be repaired. Any caller should compare
@@ -2484,7 +2523,7 @@ static int repair_inode_item(struct btrfs_root *root,
ret = 0;
goto out;
}
- if (!(err & NBYTES_ERROR)) {
+ if (!(err & NBYTES_ERROR) && !(err & ISIZE_ERROR)) {
warning("root %llu INODE[%llu] have error(s) can't repair, error : %d",
root->objectid, info->ino, err);
/* can't fix any errors, ret should be positive */
@@ -2505,6 +2544,13 @@ static int repair_inode_item(struct btrfs_root *root,
else if (ret < 0)
goto out;
}
+ if (err & ISIZE_ERROR) {
+ ret = repair_inode_isize_lowmem(trans, root, info);
+ if (ret == 0)
+ err &= ~ISIZE_ERROR;
+ else if (ret < 0)
+ goto out;
+ }
if (err != info->err) {
info->err = err;
@@ -5039,6 +5085,7 @@ out:
if (isize != size) {
err |= ISIZE_ERROR;
+ info->isize = size;
error("root %llu DIR INODE [%llu] size(%llu) not equal to %llu",
root->objectid, inode_id, isize, size);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes fix in lowmem
2017-01-09 5:38 [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes fix in lowmem Su Yue
2017-01-09 5:38 ` [PATCH 2/2] btrfs-progs: cmds-check.c: supports inode isize " Su Yue
@ 2017-01-17 15:40 ` David Sterba
2017-01-19 8:11 ` Su Yue
1 sibling, 1 reply; 5+ messages in thread
From: David Sterba @ 2017-01-17 15:40 UTC (permalink / raw)
To: Su Yue; +Cc: linux-btrfs
Hi,
I have some comments, see below.
On Mon, Jan 09, 2017 at 01:38:07PM +0800, Su Yue wrote:
> Added 'repair_inode_item' which dispatches functions such as
> 'repair_inode__nbytes_lowmem' to correct errors and
> 'struct inode_item_fix_info' to store correct values and errors.
>
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
> cmds-check.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 152 insertions(+), 9 deletions(-)
>
> diff --git a/cmds-check.c b/cmds-check.c
> index 1dba298..567ca80 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -371,6 +371,17 @@ struct root_item_info {
> };
>
> /*
> + * Use inode_item_fix_info as function check_inode_item's arg.
> + */
> +struct inode_item_fix_info {
> + u64 ino;
> + u64 isize;
> + u64 nbytes;
> +
> + int err;
> +};
> +
> +/*
> * Error bit for low memory mode check.
> *
> * Currently no caller cares about it yet. Just internal use for error
> @@ -1866,13 +1877,16 @@ struct node_refs {
> static int update_nodes_refs(struct btrfs_root *root, u64 bytenr,
> struct node_refs *nrefs, u64 level);
> static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
> - unsigned int ext_ref);
> -
> + unsigned int ext_ref,
> + struct inode_item_fix_info *info);
> +static int repair_inode_item(struct btrfs_root *root,
> + struct inode_item_fix_info *info);
> static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
> struct node_refs *nrefs, int *level, int ext_ref)
> {
> struct extent_buffer *cur = path->nodes[0];
> struct btrfs_key key;
> + struct inode_item_fix_info info;
> u64 cur_bytenr;
> u32 nritems;
> u64 first_ino = 0;
> @@ -1881,6 +1895,7 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
> int ret = 0; /* Final return value */
> int err = 0; /* Positive error bitmap */
>
> + memset(&info, 0, sizeof(info));
> cur_bytenr = cur->start;
>
> /* skip to first inode item or the first inode number change */
> @@ -1900,8 +1915,26 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
> path->slots[0] = i;
>
> again:
> - err |= check_inode_item(root, path, ext_ref);
> + err |= check_inode_item(root, path, ext_ref, &info);
> +
> + if (repair && (err & ~LAST_ITEM)) {
> + ret = repair_inode_item(root, &info);
>
> + if (ret < 0)
> + goto out;
> + /*
> + * if some errors was repaired, path shall be searched
> + * again since path has been changed
> + */
> + if (ret == 0) {
> + btrfs_item_key_to_cpu(path->nodes[0], &key,
> + path->slots[0]);
> + btrfs_release_path(path);
> + btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +
> + cur = path->nodes[0];
> + }
> + }
> if (err & LAST_ITEM)
> goto out;
>
> @@ -2211,7 +2244,8 @@ out:
> }
>
> static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
> - unsigned int ext_ref);
> + unsigned int ext_ref,
> + struct inode_item_fix_info *info);
>
> static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
> int *level, struct node_refs *nrefs, int ext_ref)
> @@ -2293,7 +2327,7 @@ static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
> }
>
> ret = check_child_node(root, cur, path->slots[*level], next);
> - if (ret < 0)
> + if (ret < 0)
> break;
>
> if (btrfs_is_leaf(next))
> @@ -2383,6 +2417,105 @@ out:
> return ret;
> }
>
> +/*
> + * Set inode's nbytes to correct value in @info
> + *
> + * Returns <0 means on error
> + * Returns 0 means successful repair
> + */
> +static int repair_inode_nbytes_lowmem(struct btrfs_trans_handle *trans,
> + struct btrfs_root *root,
> + struct inode_item_fix_info *info)
> +{
> + struct btrfs_inode_item *ei;
> + struct btrfs_key key;
> + struct btrfs_path path;
> + int ret;
> +
> + ASSERT(info);
> + key.objectid = info->ino;
> + key.type = BTRFS_INODE_ITEM_KEY;
> + key.offset = 0;
The path init call is missing here.
> +
> + ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
> + if (ret < 0)
> + goto out;
> + if (ret > 0) {
> + ret = -ENOENT;
> + goto out;
> + }
> +
> + ei = btrfs_item_ptr(path.nodes[0], path.slots[0],
> + struct btrfs_inode_item);
> + btrfs_set_inode_nbytes(path.nodes[0], ei, info->nbytes);
> + btrfs_mark_buffer_dirty(path.nodes[0]);
> + printf("reset nbytes for inode %llu root %llu\n", info->ino,
> + root->root_key.objectid);
> +out:
> + btrfs_release_path(&path);
> + return ret;
> +}
> +
> +/*
> + * repair_inode_item - repair inode item errors
> + *
> + * Repair the inode item if error can be repaired. Any caller should compare
> + * @info->err with the before, if err diffs, some contexts should be notified.
> + *
> + * @root: subvolume_root
> + * @info: contains correct values and error
> + * when it returns, @info stores the unrepairable errors
> + *
> + * Returns < 0 represents repair function error
> + * Returns 0 represents have fixed some errors or no error at all
> + * Returns > 0 represents it can't fix any error
> + */
> +static int repair_inode_item(struct btrfs_root *root,
> + struct inode_item_fix_info *info)
> +{
> + struct btrfs_trans_handle *trans;
> + int ret = 0;
> + int err;
> +
> + ASSERT(info);
> + err = info->err;
> + if (!err) {
> + /* nothing to repair */
> + ret = 0;
> + goto out;
> + }
> + if (!(err & NBYTES_ERROR)) {
> + warning("root %llu INODE[%llu] have error(s) can't repair, error : %d",
> + root->objectid, info->ino, err);
> + /* can't fix any errors, ret should be positive */
> + ret = err;
> + goto out;
> + }
> +
> + trans = btrfs_start_transaction(root, 1);
> + if (IS_ERR(trans)) {
> + ret = PTR_ERR(trans);
> + goto out;
> + }
> +
> + if (err & NBYTES_ERROR) {
> + ret = repair_inode_nbytes_lowmem(trans, root, info);
> + if (ret == 0)
> + err &= ~NBYTES_ERROR;
> + else if (ret < 0)
> + goto out;
> + }
> +
> + if (err != info->err) {
> + info->err = err;
> + ret = 0;
> + }
> +
> + btrfs_commit_transaction(trans, root);
> +out:
> + return ret;
> +}
> +
> static int repair_inode_isize(struct btrfs_trans_handle *trans,
> struct btrfs_root *root, struct btrfs_path *path,
> struct inode_record *rec)
> @@ -4781,7 +4914,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
> * Return >0 for error or hit the traversal is done(by error bitmap)
> */
> static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
> - unsigned int ext_ref)
> + unsigned int ext_ref,
> + struct inode_item_fix_info *info)
> {
> struct extent_buffer *node;
> struct btrfs_inode_item *ii;
> @@ -4821,6 +4955,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
> dir = imode_to_type(mode) == BTRFS_FT_DIR;
> nlink = btrfs_inode_nlink(node, ii);
> nodatasum = btrfs_inode_flags(node, ii) & BTRFS_INODE_NODATASUM;
> + info->ino = inode_id;
>
> while (1) {
> ret = btrfs_next_item(root, path);
> @@ -4924,11 +5059,13 @@ out:
>
> if (nbytes != extent_size) {
> err |= NBYTES_ERROR;
> + info->nbytes = extent_size;
> error("root %llu INODE[%llu] nbytes(%llu) not equal to extent_size(%llu)",
> root->objectid, inode_id, nbytes, extent_size);
The error level does not seem appropriate here, as we're fixing
something here, or at least it appears to me as such.
> }
> }
>
> + info->err = err & ~LAST_ITEM;
> return err;
> }
>
> @@ -4936,9 +5073,11 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
> {
> struct btrfs_path path;
> struct btrfs_key key;
> + struct inode_item_fix_info info;
> int err = 0;
> int ret;
>
> + memset(&info, 0, sizeof(info));
> btrfs_init_path(&path);
> key.objectid = BTRFS_FIRST_FREE_OBJECTID;
> key.type = BTRFS_INODE_ITEM_KEY;
> @@ -4952,12 +5091,17 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
> err |= INODE_ITEM_MISSING;
> }
>
> - err |= check_inode_item(root, &path, ext_ref);
> + err |= check_inode_item(root, &path, ext_ref, &info);
> err &= ~LAST_ITEM;
> if (err && !ret)
> ret = -EIO;
> out:
> btrfs_release_path(&path);
> + if (repair) {
> + ret = repair_inode_item(root, &info);
> + if (!info.err)
> + ret = 0;
> + }
> return ret;
> }
>
> @@ -12722,8 +12866,7 @@ int cmd_check(int argc, char **argv)
> * Not supported yet
> */
> if (repair && check_mode == CHECK_MODE_LOWMEM) {
> - error("low memory mode doesn't support repair yet");
> - exit(1);
> + error("low memory mode supports repair partailly");
This should change to a warning.
> }
>
> radix_tree_init();
I'm a bit lost in the check code, the patch looks good in general, but I
could have missed some corner case. Is this code exercised by some
existing code? (The low-mem mode could be now tested by the local
override flags documented in tests/README).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: cmds-check.c: supports inode isize fix in lowmem
2017-01-09 5:38 ` [PATCH 2/2] btrfs-progs: cmds-check.c: supports inode isize " Su Yue
@ 2017-01-17 15:46 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2017-01-17 15:46 UTC (permalink / raw)
To: Su Yue; +Cc: linux-btrfs
On Mon, Jan 09, 2017 at 01:38:08PM +0800, Su Yue wrote:
> Add a function 'repair_inode_isize' to support inode isize repair.
Similar comments to this patch, missng path init and the error message
level.
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
> cmds-check.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/cmds-check.c b/cmds-check.c
> index 567ca80..088c0d8 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -2457,6 +2457,45 @@ out:
> }
>
> /*
> + * Set inode's isize to correct value in @info
Please make it more detailed why the new value is correct one.
> + *
> + * Returns <0 means on error
> + * Returns 0 means successful repair
> + */
> +static int repair_inode_isize_lowmem(struct btrfs_trans_handle *trans,
> + struct btrfs_root *root,
> + struct inode_item_fix_info *info)
> +{
> + struct btrfs_inode_item *ei;
'ei' looks like a copy-paste from some code that uses extent item, if
the variable name should be a mnemonic, so please use 'ii'.
> + struct btrfs_key key;
> + struct btrfs_path path;
> + int ret;
> +
> + ASSERT(info);
> + key.objectid = info->ino;
> + key.type = BTRFS_INODE_ITEM_KEY;
> + key.offset = 0;
> +
> + ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
> + if (ret < 0)
> + goto out;
> + if (ret > 0) {
> + ret = -ENOENT;
> + goto out;
> + }
> +
> + ei = btrfs_item_ptr(path.nodes[0], path.slots[0],
> + struct btrfs_inode_item);
> + btrfs_set_inode_size(path.nodes[0], ei, info->isize);
> + btrfs_mark_buffer_dirty(path.nodes[0]);
> + printf("reset isize for inode %llu root %llu\n", info->ino,
> + root->root_key.objectid);
> +out:
> + btrfs_release_path(&path);
> + return ret;
> +}
> +
> +/*
> * repair_inode_item - repair inode item errors
> *
> * Repair the inode item if error can be repaired. Any caller should compare
> @@ -2484,7 +2523,7 @@ static int repair_inode_item(struct btrfs_root *root,
> ret = 0;
> goto out;
> }
> - if (!(err & NBYTES_ERROR)) {
> + if (!(err & NBYTES_ERROR) && !(err & ISIZE_ERROR)) {
> warning("root %llu INODE[%llu] have error(s) can't repair, error : %d",
> root->objectid, info->ino, err);
> /* can't fix any errors, ret should be positive */
> @@ -2505,6 +2544,13 @@ static int repair_inode_item(struct btrfs_root *root,
> else if (ret < 0)
> goto out;
> }
> + if (err & ISIZE_ERROR) {
> + ret = repair_inode_isize_lowmem(trans, root, info);
> + if (ret == 0)
> + err &= ~ISIZE_ERROR;
> + else if (ret < 0)
> + goto out;
> + }
>
> if (err != info->err) {
> info->err = err;
> @@ -5039,6 +5085,7 @@ out:
>
> if (isize != size) {
> err |= ISIZE_ERROR;
> + info->isize = size;
> error("root %llu DIR INODE [%llu] size(%llu) not equal to %llu",
> root->objectid, inode_id, isize, size);
> }
> --
> 2.11.0
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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] 5+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes fix in lowmem
2017-01-17 15:40 ` [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes " David Sterba
@ 2017-01-19 8:11 ` Su Yue
0 siblings, 0 replies; 5+ messages in thread
From: Su Yue @ 2017-01-19 8:11 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs, quwenruo
Hi,
The test flag override way only runs all tests in lowmem mode or not,
but can't decide one test repair or not.
I have some ideas below:
1.Create a hidden empty file under the test dir which need to be
repaired.Edit tests/common.local:_skip_spec() to judge repair or not by
the existence of hidden file.
2.Or just edit a test.sh under the test dir.I test my patches in this
way but may the way is not grace enough.
I'm wondering which one is perferred.
Thanks
Su
On 01/17/2017 11:40 PM, David Sterba wrote:
> Hi,
>
> I have some comments, see below.
>
> On Mon, Jan 09, 2017 at 01:38:07PM +0800, Su Yue wrote:
>> Added 'repair_inode_item' which dispatches functions such as
>> 'repair_inode__nbytes_lowmem' to correct errors and
>> 'struct inode_item_fix_info' to store correct values and errors.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>> cmds-check.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 152 insertions(+), 9 deletions(-)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index 1dba298..567ca80 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -371,6 +371,17 @@ struct root_item_info {
>> };
>>
>> /*
>> + * Use inode_item_fix_info as function check_inode_item's arg.
>> + */
>> +struct inode_item_fix_info {
>> + u64 ino;
>> + u64 isize;
>> + u64 nbytes;
>> +
>> + int err;
>> +};
>> +
>> +/*
>> * Error bit for low memory mode check.
>> *
>> * Currently no caller cares about it yet. Just internal use for error
>> @@ -1866,13 +1877,16 @@ struct node_refs {
>> static int update_nodes_refs(struct btrfs_root *root, u64 bytenr,
>> struct node_refs *nrefs, u64 level);
>> static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>> - unsigned int ext_ref);
>> -
>> + unsigned int ext_ref,
>> + struct inode_item_fix_info *info);
>> +static int repair_inode_item(struct btrfs_root *root,
>> + struct inode_item_fix_info *info);
>> static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
>> struct node_refs *nrefs, int *level, int ext_ref)
>> {
>> struct extent_buffer *cur = path->nodes[0];
>> struct btrfs_key key;
>> + struct inode_item_fix_info info;
>> u64 cur_bytenr;
>> u32 nritems;
>> u64 first_ino = 0;
>> @@ -1881,6 +1895,7 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
>> int ret = 0; /* Final return value */
>> int err = 0; /* Positive error bitmap */
>>
>> + memset(&info, 0, sizeof(info));
>> cur_bytenr = cur->start;
>>
>> /* skip to first inode item or the first inode number change */
>> @@ -1900,8 +1915,26 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
>> path->slots[0] = i;
>>
>> again:
>> - err |= check_inode_item(root, path, ext_ref);
>> + err |= check_inode_item(root, path, ext_ref, &info);
>> +
>> + if (repair && (err & ~LAST_ITEM)) {
>> + ret = repair_inode_item(root, &info);
>>
>> + if (ret < 0)
>> + goto out;
>> + /*
>> + * if some errors was repaired, path shall be searched
>> + * again since path has been changed
>> + */
>> + if (ret == 0) {
>> + btrfs_item_key_to_cpu(path->nodes[0], &key,
>> + path->slots[0]);
>> + btrfs_release_path(path);
>> + btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +
>> + cur = path->nodes[0];
>> + }
>> + }
>> if (err & LAST_ITEM)
>> goto out;
>>
>> @@ -2211,7 +2244,8 @@ out:
>> }
>>
>> static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>> - unsigned int ext_ref);
>> + unsigned int ext_ref,
>> + struct inode_item_fix_info *info);
>>
>> static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
>> int *level, struct node_refs *nrefs, int ext_ref)
>> @@ -2293,7 +2327,7 @@ static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
>> }
>>
>> ret = check_child_node(root, cur, path->slots[*level], next);
>> - if (ret < 0)
>> + if (ret < 0)
>> break;
>>
>> if (btrfs_is_leaf(next))
>> @@ -2383,6 +2417,105 @@ out:
>> return ret;
>> }
>>
>> +/*
>> + * Set inode's nbytes to correct value in @info
>> + *
>> + * Returns <0 means on error
>> + * Returns 0 means successful repair
>> + */
>> +static int repair_inode_nbytes_lowmem(struct btrfs_trans_handle *trans,
>> + struct btrfs_root *root,
>> + struct inode_item_fix_info *info)
>> +{
>> + struct btrfs_inode_item *ei;
>> + struct btrfs_key key;
>> + struct btrfs_path path;
>> + int ret;
>> +
>> + ASSERT(info);
>> + key.objectid = info->ino;
>> + key.type = BTRFS_INODE_ITEM_KEY;
>> + key.offset = 0;
>
> The path init call is missing here.
>
>> +
>> + ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
>> + if (ret < 0)
>> + goto out;
>> + if (ret > 0) {
>> + ret = -ENOENT;
>> + goto out;
>> + }
>> +
>> + ei = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> + struct btrfs_inode_item);
>> + btrfs_set_inode_nbytes(path.nodes[0], ei, info->nbytes);
>> + btrfs_mark_buffer_dirty(path.nodes[0]);
>> + printf("reset nbytes for inode %llu root %llu\n", info->ino,
>> + root->root_key.objectid);
>> +out:
>> + btrfs_release_path(&path);
>> + return ret;
>> +}
>> +
>> +/*
>> + * repair_inode_item - repair inode item errors
>> + *
>> + * Repair the inode item if error can be repaired. Any caller should compare
>> + * @info->err with the before, if err diffs, some contexts should be notified.
>> + *
>> + * @root: subvolume_root
>> + * @info: contains correct values and error
>> + * when it returns, @info stores the unrepairable errors
>> + *
>> + * Returns < 0 represents repair function error
>> + * Returns 0 represents have fixed some errors or no error at all
>> + * Returns > 0 represents it can't fix any error
>> + */
>> +static int repair_inode_item(struct btrfs_root *root,
>> + struct inode_item_fix_info *info)
>> +{
>> + struct btrfs_trans_handle *trans;
>> + int ret = 0;
>> + int err;
>> +
>> + ASSERT(info);
>> + err = info->err;
>> + if (!err) {
>> + /* nothing to repair */
>> + ret = 0;
>> + goto out;
>> + }
>> + if (!(err & NBYTES_ERROR)) {
>> + warning("root %llu INODE[%llu] have error(s) can't repair, error : %d",
>> + root->objectid, info->ino, err);
>> + /* can't fix any errors, ret should be positive */
>> + ret = err;
>> + goto out;
>> + }
>> +
>> + trans = btrfs_start_transaction(root, 1);
>> + if (IS_ERR(trans)) {
>> + ret = PTR_ERR(trans);
>> + goto out;
>> + }
>> +
>> + if (err & NBYTES_ERROR) {
>> + ret = repair_inode_nbytes_lowmem(trans, root, info);
>> + if (ret == 0)
>> + err &= ~NBYTES_ERROR;
>> + else if (ret < 0)
>> + goto out;
>> + }
>> +
>> + if (err != info->err) {
>> + info->err = err;
>> + ret = 0;
>> + }
>> +
>> + btrfs_commit_transaction(trans, root);
>> +out:
>> + return ret;
>> +}
>> +
>> static int repair_inode_isize(struct btrfs_trans_handle *trans,
>> struct btrfs_root *root, struct btrfs_path *path,
>> struct inode_record *rec)
>> @@ -4781,7 +4914,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>> * Return >0 for error or hit the traversal is done(by error bitmap)
>> */
>> static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>> - unsigned int ext_ref)
>> + unsigned int ext_ref,
>> + struct inode_item_fix_info *info)
>> {
>> struct extent_buffer *node;
>> struct btrfs_inode_item *ii;
>> @@ -4821,6 +4955,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>> dir = imode_to_type(mode) == BTRFS_FT_DIR;
>> nlink = btrfs_inode_nlink(node, ii);
>> nodatasum = btrfs_inode_flags(node, ii) & BTRFS_INODE_NODATASUM;
>> + info->ino = inode_id;
>>
>> while (1) {
>> ret = btrfs_next_item(root, path);
>> @@ -4924,11 +5059,13 @@ out:
>>
>> if (nbytes != extent_size) {
>> err |= NBYTES_ERROR;
>> + info->nbytes = extent_size;
>> error("root %llu INODE[%llu] nbytes(%llu) not equal to extent_size(%llu)",
>> root->objectid, inode_id, nbytes, extent_size);
>
> The error level does not seem appropriate here, as we're fixing
> something here, or at least it appears to me as such.
>
>> }
>> }
>>
>> + info->err = err & ~LAST_ITEM;
>> return err;
>> }
>>
>> @@ -4936,9 +5073,11 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
>> {
>> struct btrfs_path path;
>> struct btrfs_key key;
>> + struct inode_item_fix_info info;
>> int err = 0;
>> int ret;
>>
>> + memset(&info, 0, sizeof(info));
>> btrfs_init_path(&path);
>> key.objectid = BTRFS_FIRST_FREE_OBJECTID;
>> key.type = BTRFS_INODE_ITEM_KEY;
>> @@ -4952,12 +5091,17 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
>> err |= INODE_ITEM_MISSING;
>> }
>>
>> - err |= check_inode_item(root, &path, ext_ref);
>> + err |= check_inode_item(root, &path, ext_ref, &info);
>> err &= ~LAST_ITEM;
>> if (err && !ret)
>> ret = -EIO;
>> out:
>> btrfs_release_path(&path);
>> + if (repair) {
>> + ret = repair_inode_item(root, &info);
>> + if (!info.err)
>> + ret = 0;
>> + }
>> return ret;
>> }
>>
>> @@ -12722,8 +12866,7 @@ int cmd_check(int argc, char **argv)
>> * Not supported yet
>> */
>> if (repair && check_mode == CHECK_MODE_LOWMEM) {
>> - error("low memory mode doesn't support repair yet");
>> - exit(1);
>> + error("low memory mode supports repair partailly");
>
> This should change to a warning.
>
>> }
>>
>> radix_tree_init();
>
> I'm a bit lost in the check code, the patch looks good in general, but I
> could have missed some corner case. Is this code exercised by some
> existing code? (The low-mem mode could be now tested by the local
> override flags documented in tests/README).
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-19 8:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-09 5:38 [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes fix in lowmem Su Yue
2017-01-09 5:38 ` [PATCH 2/2] btrfs-progs: cmds-check.c: supports inode isize " Su Yue
2017-01-17 15:46 ` David Sterba
2017-01-17 15:40 ` [PATCH 1/2] btrfs-progs: cmds-check.c: supports inode nbytes " David Sterba
2017-01-19 8:11 ` Su Yue
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.