| Message ID | 20190924081120.6283-3-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: Add check and repair for invalid inode generation | expand |
On 24.09.19 г. 11:11 ч., Qu Wenruo wrote: > There are at least two bug reports of kernel tree-checker complaining > about invalid inode generation. > > All offending inodes seem to be caused by old kernel around 2014, with > inode generation overflow. > > So add such check and repair ability to lowmem mode check first. > > This involves: > - Calculate the inode generation upper limit > Unlike the original mode context, we don't have anyway to determine if Perhaps you meant "lowmem mode context" since in lowmem you deduce whether it's a log-tree inode or not. > this inode belongs to log tree. > So we use super_generation + 1 as upper limit, just like what we did > in kernel tree checker. > > - Check if the inode generation is larger than the upper limit > > - Repair by resetting inode generation to current transaction > generation > The difference is, in original mode, we have a common trans handle for > all repair and reset path for each repair. > > Reported-by: Charles Wright <charles.v.wright@gmail.com> > Signed-off-by: Qu Wenruo <wqu@suse.com> I tested that code with the image provided and it does work as expected as well as it's fairly obvious what's happening. So: Reviewed-by: Nikolay Borisov <nborisov@suse.com> Tested-by: Nikolay Borisov <nborisov@suse.com> > --- > check/main.c | 50 ++++++++++++++++++++++++++++++++++++++++++- > check/mode-original.h | 1 + > 2 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/check/main.c b/check/main.c > index c2d0f394..018707c8 100644 > --- a/check/main.c > +++ b/check/main.c > @@ -604,6 +604,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec) > if (errors & I_ERR_INVALID_IMODE) > fprintf(stderr, ", invalid inode mode bit 0%o", > rec->imode & ~07777); > + if (errors & I_ERR_INVALID_GEN) > + fprintf(stderr, ", invalid inode generation"); > fprintf(stderr, "\n"); > > /* Print the holes if needed */ > @@ -857,6 +859,7 @@ static int process_inode_item(struct extent_buffer *eb, > { > struct inode_record *rec; > struct btrfs_inode_item *item; > + u64 gen_uplimit; > u64 flags; > > rec = active_node->current; > @@ -879,6 +882,13 @@ static int process_inode_item(struct extent_buffer *eb, > if (S_ISLNK(rec->imode) && > flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) > rec->errors |= I_ERR_ODD_INODE_FLAGS; > + /* > + * We don't have accurate root info to determine the correct > + * inode generation uplimit, use super_generation + 1 anyway > + */ > + gen_uplimit = btrfs_super_generation(global_info->super_copy) + 1; > + if (btrfs_inode_generation(eb, item) > gen_uplimit) > + rec->errors |= I_ERR_INVALID_GEN; > maybe_free_inode_rec(&active_node->inode_cache, rec); > return 0; > } > @@ -2774,6 +2784,41 @@ static int repair_imode_original(struct btrfs_trans_handle *trans, > return ret; > } > > +static int repair_inode_gen_original(struct btrfs_trans_handle *trans, > + struct btrfs_root *root, > + struct btrfs_path *path, > + struct inode_record *rec) > +{ > + struct btrfs_inode_item *ii; > + struct btrfs_key key; > + int ret; > + > + key.objectid = rec->ino; > + key.offset = 0; > + key.type = BTRFS_INODE_ITEM_KEY; > + > + ret = btrfs_search_slot(trans, root, &key, path, 0, 1); > + if (ret > 0) { > + ret = -ENOENT; > + error("no inode item found for ino %llu", rec->ino); > + return ret; > + } > + if (ret < 0) { > + errno = -ret; > + error("failed to search inode item for ino %llu: %m", rec->ino); > + return ret; > + } > + ii = btrfs_item_ptr(path->nodes[0], path->slots[0], > + struct btrfs_inode_item); > + btrfs_set_inode_generation(path->nodes[0], ii, trans->transid); > + btrfs_mark_buffer_dirty(path->nodes[0]); > + btrfs_release_path(path); > + printf("resetting inode generation to %llu for ino %llu\n", > + trans->transid, rec->ino); > + rec->errors &= ~I_ERR_INVALID_GEN; > + return 0; > +} > + > static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) > { > struct btrfs_trans_handle *trans; > @@ -2794,7 +2839,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) > I_ERR_FILE_NBYTES_WRONG | > I_ERR_INLINE_RAM_BYTES_WRONG | > I_ERR_MISMATCH_DIR_HASH | > - I_ERR_UNALIGNED_EXTENT_REC))) > + I_ERR_UNALIGNED_EXTENT_REC | > + I_ERR_INVALID_GEN))) > return rec->errors; > > /* > @@ -2829,6 +2875,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) > ret = repair_unaligned_extent_recs(trans, root, &path, rec); > if (!ret && rec->errors & I_ERR_INVALID_IMODE) > ret = repair_imode_original(trans, root, &path, rec); > + if (!ret && rec->errors & I_ERR_INVALID_GEN) > + ret = repair_inode_gen_original(trans, root, &path, rec); > btrfs_commit_transaction(trans, root); > btrfs_release_path(&path); > return ret; > diff --git a/check/mode-original.h b/check/mode-original.h > index 78d6bb30..b075a95c 100644 > --- a/check/mode-original.h > +++ b/check/mode-original.h > @@ -185,6 +185,7 @@ struct unaligned_extent_rec_t { > #define I_ERR_INLINE_RAM_BYTES_WRONG (1 << 17) > #define I_ERR_MISMATCH_DIR_HASH (1 << 18) > #define I_ERR_INVALID_IMODE (1 << 19) > +#define I_ERR_INVALID_GEN (1 << 20) > > struct inode_record { > struct list_head backrefs; >
On 2019/9/30 下午4:41, Nikolay Borisov wrote: > > > On 24.09.19 г. 11:11 ч., Qu Wenruo wrote: >> There are at least two bug reports of kernel tree-checker complaining >> about invalid inode generation. >> >> All offending inodes seem to be caused by old kernel around 2014, with >> inode generation overflow. >> >> So add such check and repair ability to lowmem mode check first. >> >> This involves: >> - Calculate the inode generation upper limit >> Unlike the original mode context, we don't have anyway to determine if > > Perhaps you meant "lowmem mode context" since in lowmem you deduce > whether it's a log-tree inode or not. Oh, right. Would you mind to fix this in commit time, David? > >> this inode belongs to log tree. >> So we use super_generation + 1 as upper limit, just like what we did >> in kernel tree checker. >> >> - Check if the inode generation is larger than the upper limit >> >> - Repair by resetting inode generation to current transaction >> generation >> The difference is, in original mode, we have a common trans handle for >> all repair and reset path for each repair. >> >> Reported-by: Charles Wright <charles.v.wright@gmail.com> >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > I tested that code with the image provided and it does work as expected > as well as it's fairly obvious what's happening. So: > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> > Tested-by: Nikolay Borisov <nborisov@suse.com> Thanks for the review and test. Thanks, Qu > >> --- >> check/main.c | 50 ++++++++++++++++++++++++++++++++++++++++++- >> check/mode-original.h | 1 + >> 2 files changed, 50 insertions(+), 1 deletion(-) >> >> diff --git a/check/main.c b/check/main.c >> index c2d0f394..018707c8 100644 >> --- a/check/main.c >> +++ b/check/main.c >> @@ -604,6 +604,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec) >> if (errors & I_ERR_INVALID_IMODE) >> fprintf(stderr, ", invalid inode mode bit 0%o", >> rec->imode & ~07777); >> + if (errors & I_ERR_INVALID_GEN) >> + fprintf(stderr, ", invalid inode generation"); >> fprintf(stderr, "\n"); >> >> /* Print the holes if needed */ >> @@ -857,6 +859,7 @@ static int process_inode_item(struct extent_buffer *eb, >> { >> struct inode_record *rec; >> struct btrfs_inode_item *item; >> + u64 gen_uplimit; >> u64 flags; >> >> rec = active_node->current; >> @@ -879,6 +882,13 @@ static int process_inode_item(struct extent_buffer *eb, >> if (S_ISLNK(rec->imode) && >> flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) >> rec->errors |= I_ERR_ODD_INODE_FLAGS; >> + /* >> + * We don't have accurate root info to determine the correct >> + * inode generation uplimit, use super_generation + 1 anyway >> + */ >> + gen_uplimit = btrfs_super_generation(global_info->super_copy) + 1; >> + if (btrfs_inode_generation(eb, item) > gen_uplimit) >> + rec->errors |= I_ERR_INVALID_GEN; >> maybe_free_inode_rec(&active_node->inode_cache, rec); >> return 0; >> } >> @@ -2774,6 +2784,41 @@ static int repair_imode_original(struct btrfs_trans_handle *trans, >> return ret; >> } >> >> +static int repair_inode_gen_original(struct btrfs_trans_handle *trans, >> + struct btrfs_root *root, >> + struct btrfs_path *path, >> + struct inode_record *rec) >> +{ >> + struct btrfs_inode_item *ii; >> + struct btrfs_key key; >> + int ret; >> + >> + key.objectid = rec->ino; >> + key.offset = 0; >> + key.type = BTRFS_INODE_ITEM_KEY; >> + >> + ret = btrfs_search_slot(trans, root, &key, path, 0, 1); >> + if (ret > 0) { >> + ret = -ENOENT; >> + error("no inode item found for ino %llu", rec->ino); >> + return ret; >> + } >> + if (ret < 0) { >> + errno = -ret; >> + error("failed to search inode item for ino %llu: %m", rec->ino); >> + return ret; >> + } >> + ii = btrfs_item_ptr(path->nodes[0], path->slots[0], >> + struct btrfs_inode_item); >> + btrfs_set_inode_generation(path->nodes[0], ii, trans->transid); >> + btrfs_mark_buffer_dirty(path->nodes[0]); >> + btrfs_release_path(path); >> + printf("resetting inode generation to %llu for ino %llu\n", >> + trans->transid, rec->ino); >> + rec->errors &= ~I_ERR_INVALID_GEN; >> + return 0; >> +} >> + >> static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) >> { >> struct btrfs_trans_handle *trans; >> @@ -2794,7 +2839,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) >> I_ERR_FILE_NBYTES_WRONG | >> I_ERR_INLINE_RAM_BYTES_WRONG | >> I_ERR_MISMATCH_DIR_HASH | >> - I_ERR_UNALIGNED_EXTENT_REC))) >> + I_ERR_UNALIGNED_EXTENT_REC | >> + I_ERR_INVALID_GEN))) >> return rec->errors; >> >> /* >> @@ -2829,6 +2875,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) >> ret = repair_unaligned_extent_recs(trans, root, &path, rec); >> if (!ret && rec->errors & I_ERR_INVALID_IMODE) >> ret = repair_imode_original(trans, root, &path, rec); >> + if (!ret && rec->errors & I_ERR_INVALID_GEN) >> + ret = repair_inode_gen_original(trans, root, &path, rec); >> btrfs_commit_transaction(trans, root); >> btrfs_release_path(&path); >> return ret; >> diff --git a/check/mode-original.h b/check/mode-original.h >> index 78d6bb30..b075a95c 100644 >> --- a/check/mode-original.h >> +++ b/check/mode-original.h >> @@ -185,6 +185,7 @@ struct unaligned_extent_rec_t { >> #define I_ERR_INLINE_RAM_BYTES_WRONG (1 << 17) >> #define I_ERR_MISMATCH_DIR_HASH (1 << 18) >> #define I_ERR_INVALID_IMODE (1 << 19) >> +#define I_ERR_INVALID_GEN (1 << 20) >> >> struct inode_record { >> struct list_head backrefs; >>
diff --git a/check/main.c b/check/main.c index c2d0f394..018707c8 100644 --- a/check/main.c +++ b/check/main.c @@ -604,6 +604,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec) if (errors & I_ERR_INVALID_IMODE) fprintf(stderr, ", invalid inode mode bit 0%o", rec->imode & ~07777); + if (errors & I_ERR_INVALID_GEN) + fprintf(stderr, ", invalid inode generation"); fprintf(stderr, "\n"); /* Print the holes if needed */ @@ -857,6 +859,7 @@ static int process_inode_item(struct extent_buffer *eb, { struct inode_record *rec; struct btrfs_inode_item *item; + u64 gen_uplimit; u64 flags; rec = active_node->current; @@ -879,6 +882,13 @@ static int process_inode_item(struct extent_buffer *eb, if (S_ISLNK(rec->imode) && flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) rec->errors |= I_ERR_ODD_INODE_FLAGS; + /* + * We don't have accurate root info to determine the correct + * inode generation uplimit, use super_generation + 1 anyway + */ + gen_uplimit = btrfs_super_generation(global_info->super_copy) + 1; + if (btrfs_inode_generation(eb, item) > gen_uplimit) + rec->errors |= I_ERR_INVALID_GEN; maybe_free_inode_rec(&active_node->inode_cache, rec); return 0; } @@ -2774,6 +2784,41 @@ static int repair_imode_original(struct btrfs_trans_handle *trans, return ret; } +static int repair_inode_gen_original(struct btrfs_trans_handle *trans, + struct btrfs_root *root, + struct btrfs_path *path, + struct inode_record *rec) +{ + struct btrfs_inode_item *ii; + struct btrfs_key key; + int ret; + + key.objectid = rec->ino; + key.offset = 0; + key.type = BTRFS_INODE_ITEM_KEY; + + ret = btrfs_search_slot(trans, root, &key, path, 0, 1); + if (ret > 0) { + ret = -ENOENT; + error("no inode item found for ino %llu", rec->ino); + return ret; + } + if (ret < 0) { + errno = -ret; + error("failed to search inode item for ino %llu: %m", rec->ino); + return ret; + } + ii = btrfs_item_ptr(path->nodes[0], path->slots[0], + struct btrfs_inode_item); + btrfs_set_inode_generation(path->nodes[0], ii, trans->transid); + btrfs_mark_buffer_dirty(path->nodes[0]); + btrfs_release_path(path); + printf("resetting inode generation to %llu for ino %llu\n", + trans->transid, rec->ino); + rec->errors &= ~I_ERR_INVALID_GEN; + return 0; +} + static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) { struct btrfs_trans_handle *trans; @@ -2794,7 +2839,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) I_ERR_FILE_NBYTES_WRONG | I_ERR_INLINE_RAM_BYTES_WRONG | I_ERR_MISMATCH_DIR_HASH | - I_ERR_UNALIGNED_EXTENT_REC))) + I_ERR_UNALIGNED_EXTENT_REC | + I_ERR_INVALID_GEN))) return rec->errors; /* @@ -2829,6 +2875,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) ret = repair_unaligned_extent_recs(trans, root, &path, rec); if (!ret && rec->errors & I_ERR_INVALID_IMODE) ret = repair_imode_original(trans, root, &path, rec); + if (!ret && rec->errors & I_ERR_INVALID_GEN) + ret = repair_inode_gen_original(trans, root, &path, rec); btrfs_commit_transaction(trans, root); btrfs_release_path(&path); return ret; diff --git a/check/mode-original.h b/check/mode-original.h index 78d6bb30..b075a95c 100644 --- a/check/mode-original.h +++ b/check/mode-original.h @@ -185,6 +185,7 @@ struct unaligned_extent_rec_t { #define I_ERR_INLINE_RAM_BYTES_WRONG (1 << 17) #define I_ERR_MISMATCH_DIR_HASH (1 << 18) #define I_ERR_INVALID_IMODE (1 << 19) +#define I_ERR_INVALID_GEN (1 << 20) struct inode_record { struct list_head backrefs;
There are at least two bug reports of kernel tree-checker complaining about invalid inode generation. All offending inodes seem to be caused by old kernel around 2014, with inode generation overflow. So add such check and repair ability to lowmem mode check first. This involves: - Calculate the inode generation upper limit Unlike the original mode context, we don't have anyway to determine if this inode belongs to log tree. So we use super_generation + 1 as upper limit, just like what we did in kernel tree checker. - Check if the inode generation is larger than the upper limit - Repair by resetting inode generation to current transaction generation The difference is, in original mode, we have a common trans handle for all repair and reset path for each repair. Reported-by: Charles Wright <charles.v.wright@gmail.com> Signed-off-by: Qu Wenruo <wqu@suse.com> --- check/main.c | 50 ++++++++++++++++++++++++++++++++++++++++++- check/mode-original.h | 1 + 2 files changed, 50 insertions(+), 1 deletion(-)