* [PATCH 0/2] btrfs: tree-checker: Add checks to detect missing INODE_ITEM
@ 2019-08-26 7:40 Qu Wenruo
2019-08-26 7:40 ` [PATCH 1/2] btrfs: tree-checker: Try " Qu Wenruo
2019-08-26 7:40 ` [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF Qu Wenruo
0 siblings, 2 replies; 8+ messages in thread
From: Qu Wenruo @ 2019-08-26 7:40 UTC (permalink / raw)
To: linux-btrfs
For the following items, key->objectid is inode number:
- DIR_ITEM
- DIR_INDEX
- XATTR_ITEM
- EXTENT_DATA
- INODE_REF
So in btrfs btree, such items must have its previous item shares the
same objectid, e.g.:
(257 INODE_ITEM 0)
(257 DIR_INDEX xxx)
(257 DIR_ITEM xxx)
(258 INODE_ITEM 0)
(258 INODE_REF 0)
(258 XATTR_ITEM 0)
(258 EXTENT_DATA 0)
But if we have the following sequence, then there is definitely
something wrong, normally some INODE_ITEM is missing, like:
(257 INODE_ITEM 0)
(257 DIR_INDEX xxx)
(257 DIR_ITEM xxx)
(258 XATTR_ITEM 0) <<< objecitd suddenly changed to 258
(258 EXTENT_DATA 0)
So just by checking the previous key for above inode based key types, we
can detect missing inode item.
In this patchset, we will enhance existing check_dir_item() and
check_extent_data_item() to detect missing INODE_ITEM first, then add
INODE_REF checker.
So now we can cover the INODE_ITEM missing case in tree-checker without
much cost, but achieve the check which is normally done by btrfs-check.
(I'm already a little concerned about the fact that kernel tree-checker
is getting stronger and stronger while btrfs-progs can't fix all
problems)
Of course, there is still a limitation that the first key of a leaf
can't be verified, but we have already cover all the rest keys, which is
way better than "good enough"(TM).
Qu Wenruo (2):
btrfs: tree-checker: Try to detect missing INODE_ITEM
btrfs: tree-checker: Add check for INODE_REF
fs/btrfs/tree-checker.c | 78 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 76 insertions(+), 2 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] btrfs: tree-checker: Try to detect missing INODE_ITEM
2019-08-26 7:40 [PATCH 0/2] btrfs: tree-checker: Add checks to detect missing INODE_ITEM Qu Wenruo
@ 2019-08-26 7:40 ` Qu Wenruo
2019-08-26 11:46 ` Nikolay Borisov
2019-08-26 7:40 ` [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF Qu Wenruo
1 sibling, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2019-08-26 7:40 UTC (permalink / raw)
To: linux-btrfs
For the following items, key->objectid is inode number:
- DIR_ITEM
- DIR_INDEX
- XATTR_ITEM
- EXTENT_DATA
- INODE_REF
So in btrfs btree, such items must have its previous item shares the
same objectid, e.g.:
(257 INODE_ITEM 0)
(257 DIR_INDEX xxx)
(257 DIR_ITEM xxx)
(258 INODE_ITEM 0)
(258 INODE_REF 0)
(258 XATTR_ITEM 0)
(258 EXTENT_DATA 0)
But if we have the following sequence, then there is definitely
something wrong, normally some INODE_ITEM is missing, like:
(257 INODE_ITEM 0)
(257 DIR_INDEX xxx)
(257 DIR_ITEM xxx)
(258 XATTR_ITEM 0) <<< objecitd suddenly changed to 258
(258 EXTENT_DATA 0)
So just by checking the previous key for above inode based key types, we
can detect missing inode item.
For INODE_REF key type, the check will be added along with INODE_REF
checker.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/tree-checker.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index ccd5706199d7..636ce1b4566e 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -141,6 +141,19 @@ static int check_extent_data_item(struct extent_buffer *leaf,
return -EUCLEAN;
}
+ /*
+ * Previous key must have the same key->objectid (ino).
+ * It can be XATTR_ITEM, INODE_ITEM or just another EXTENT_DATA.
+ * But if objectids mismatch, it means we have a missing
+ * INODE_ITEM.
+ */
+ if (slot > 0 && prev_key->objectid != key->objectid) {
+ file_extent_err(leaf, slot,
+ "invalid previous key objectid, have %llu expect %llu",
+ prev_key->objectid, key->objectid);
+ return -EUCLEAN;
+ }
+
fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
@@ -299,13 +312,21 @@ static void dir_item_err(const struct extent_buffer *eb, int slot,
}
static int check_dir_item(struct extent_buffer *leaf,
- struct btrfs_key *key, int slot)
+ struct btrfs_key *key, struct btrfs_key *prev_key,
+ int slot)
{
struct btrfs_fs_info *fs_info = leaf->fs_info;
struct btrfs_dir_item *di;
u32 item_size = btrfs_item_size_nr(leaf, slot);
u32 cur = 0;
+ /* Same check as in check_extent_data_item() */
+ if (slot > 0 && prev_key->objectid != key->objectid) {
+ dir_item_err(leaf, slot,
+ "invalid previous key objectid, have %llu expect %llu",
+ prev_key->objectid, key->objectid);
+ return -EUCLEAN;
+ }
di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
while (cur < item_size) {
u32 name_len;
@@ -841,7 +862,7 @@ static int check_leaf_item(struct extent_buffer *leaf,
case BTRFS_DIR_ITEM_KEY:
case BTRFS_DIR_INDEX_KEY:
case BTRFS_XATTR_ITEM_KEY:
- ret = check_dir_item(leaf, key, slot);
+ ret = check_dir_item(leaf, key, prev_key, slot);
break;
case BTRFS_BLOCK_GROUP_ITEM_KEY:
ret = check_block_group_item(leaf, key, slot);
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF
2019-08-26 7:40 [PATCH 0/2] btrfs: tree-checker: Add checks to detect missing INODE_ITEM Qu Wenruo
2019-08-26 7:40 ` [PATCH 1/2] btrfs: tree-checker: Try " Qu Wenruo
@ 2019-08-26 7:40 ` Qu Wenruo
2019-08-26 11:45 ` Nikolay Borisov
2019-09-23 15:47 ` David Sterba
1 sibling, 2 replies; 8+ messages in thread
From: Qu Wenruo @ 2019-08-26 7:40 UTC (permalink / raw)
To: linux-btrfs
For INODE_REF we will check:
- Objectid (ino) against previous key
To detect missing INODE_ITEM.
- No overflow/padding in the data payload
Much like DIR_ITEM, but with less members to check.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/tree-checker.c | 53 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 636ce1b4566e..3ce447eb591c 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -842,6 +842,56 @@ static int check_inode_item(struct extent_buffer *leaf,
return 0;
}
+#define inode_ref_err(fs_info, eb, slot, fmt, ...) \
+ inode_item_err(fs_info, eb, slot, fmt, __VA_ARGS__)
+static int check_inode_ref(struct extent_buffer *leaf,
+ struct btrfs_key *key, struct btrfs_key *prev_key,
+ int slot)
+{
+ struct btrfs_inode_ref *iref;
+ unsigned long ptr;
+ unsigned long end;
+
+ /* namelen can't be 0, so item_size == sizeof() is also invalid */
+ if (btrfs_item_size_nr(leaf, slot) <= sizeof(*iref)) {
+ inode_ref_err(fs_info, leaf, slot,
+ "invalid item size, have %u expect (%zu, %u)",
+ btrfs_item_size_nr(leaf, slot),
+ sizeof(*iref), BTRFS_LEAF_DATA_SIZE(leaf->fs_info));
+ return -EUCLEAN;
+ }
+
+ ptr = btrfs_item_ptr_offset(leaf, slot);
+ end = ptr + btrfs_item_size_nr(leaf, slot);
+ while (ptr < end) {
+ u16 namelen;
+
+ if (ptr + sizeof(iref) > end) {
+ inode_ref_err(fs_info, leaf, slot,
+ "inode ref overflow, ptr %lu end %lu inode_ref_size %zu",
+ ptr, end, sizeof(iref));
+ return -EUCLEAN;
+ }
+
+ iref = (struct btrfs_inode_ref *)ptr;
+ namelen = btrfs_inode_ref_name_len(leaf, iref);
+ if (ptr + sizeof(*iref) + namelen > end) {
+ inode_ref_err(fs_info, leaf, slot,
+ "inode ref overflow, ptr %lu end %lu namelen %u",
+ ptr, end, namelen);
+ return -EUCLEAN;
+ }
+
+ /*
+ * NOTE: In theory we should record all found index number
+ * to find any duplicated index. But that will be too time
+ * consuming for inodes with too many hard links.
+ */
+ ptr += sizeof(*iref) + namelen;
+ }
+ return 0;
+}
+
/*
* Common point to switch the item-specific validation.
*/
@@ -864,6 +914,9 @@ static int check_leaf_item(struct extent_buffer *leaf,
case BTRFS_XATTR_ITEM_KEY:
ret = check_dir_item(leaf, key, prev_key, slot);
break;
+ case BTRFS_INODE_REF_KEY:
+ ret = check_inode_ref(leaf, key, prev_key, slot);
+ break;
case BTRFS_BLOCK_GROUP_ITEM_KEY:
ret = check_block_group_item(leaf, key, slot);
break;
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF
2019-08-26 7:40 ` [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF Qu Wenruo
@ 2019-08-26 11:45 ` Nikolay Borisov
2019-08-26 11:50 ` Qu Wenruo
2019-09-23 15:47 ` David Sterba
1 sibling, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-08-26 11:45 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 26.08.19 г. 10:40 ч., Qu Wenruo wrote:
> For INODE_REF we will check:
> - Objectid (ino) against previous key
> To detect missing INODE_ITEM.
>
> - No overflow/padding in the data payload
> Much like DIR_ITEM, but with less members to check.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/tree-checker.c | 53 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 636ce1b4566e..3ce447eb591c 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -842,6 +842,56 @@ static int check_inode_item(struct extent_buffer *leaf,
> return 0;
> }
>
> +#define inode_ref_err(fs_info, eb, slot, fmt, ...) \
> + inode_item_err(fs_info, eb, slot, fmt, __VA_ARGS__)
This define doesn't bring anything, just opencode the call to
inode_item_err directly.
> +static int check_inode_ref(struct extent_buffer *leaf,
> + struct btrfs_key *key, struct btrfs_key *prev_key,
> + int slot)
> +{
> + struct btrfs_inode_ref *iref;
> + unsigned long ptr;
> + unsigned long end;
> +
> + /* namelen can't be 0, so item_size == sizeof() is also invalid */
> + if (btrfs_item_size_nr(leaf, slot) <= sizeof(*iref)) {
> + inode_ref_err(fs_info, leaf, slot,
> + "invalid item size, have %u expect (%zu, %u)",
> + btrfs_item_size_nr(leaf, slot),
> + sizeof(*iref), BTRFS_LEAF_DATA_SIZE(leaf->fs_info));
> + return -EUCLEAN;
> + }
> +
> + ptr = btrfs_item_ptr_offset(leaf, slot);
> + end = ptr + btrfs_item_size_nr(leaf, slot);
> + while (ptr < end) {
> + u16 namelen;
> +
> + if (ptr + sizeof(iref) > end) {
> + inode_ref_err(fs_info, leaf, slot,
> + "inode ref overflow, ptr %lu end %lu inode_ref_size %zu",
> + ptr, end, sizeof(iref));
> + return -EUCLEAN;
> + }
> +
> + iref = (struct btrfs_inode_ref *)ptr;
> + namelen = btrfs_inode_ref_name_len(leaf, iref);
> + if (ptr + sizeof(*iref) + namelen > end) {
> + inode_ref_err(fs_info, leaf, slot,
> + "inode ref overflow, ptr %lu end %lu namelen %u",
> + ptr, end, namelen);
> + return -EUCLEAN;
> + }
> +
> + /*
> + * NOTE: In theory we should record all found index number
> + * to find any duplicated index. But that will be too time
> + * consuming for inodes with too many hard links.
> + */
> + ptr += sizeof(*iref) + namelen;
> + }
> + return 0;
> +}
> +
> /*
> * Common point to switch the item-specific validation.
> */
> @@ -864,6 +914,9 @@ static int check_leaf_item(struct extent_buffer *leaf,
> case BTRFS_XATTR_ITEM_KEY:
> ret = check_dir_item(leaf, key, prev_key, slot);
> break;
> + case BTRFS_INODE_REF_KEY:
> + ret = check_inode_ref(leaf, key, prev_key, slot);
> + break;
> case BTRFS_BLOCK_GROUP_ITEM_KEY:
> ret = check_block_group_item(leaf, key, slot);
> break;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] btrfs: tree-checker: Try to detect missing INODE_ITEM
2019-08-26 7:40 ` [PATCH 1/2] btrfs: tree-checker: Try " Qu Wenruo
@ 2019-08-26 11:46 ` Nikolay Borisov
0 siblings, 0 replies; 8+ messages in thread
From: Nikolay Borisov @ 2019-08-26 11:46 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 26.08.19 г. 10:40 ч., Qu Wenruo wrote:
> For the following items, key->objectid is inode number:
> - DIR_ITEM
> - DIR_INDEX
> - XATTR_ITEM
> - EXTENT_DATA
> - INODE_REF
>
> So in btrfs btree, such items must have its previous item shares the
> same objectid, e.g.:
> (257 INODE_ITEM 0)
> (257 DIR_INDEX xxx)
> (257 DIR_ITEM xxx)
> (258 INODE_ITEM 0)
> (258 INODE_REF 0)
> (258 XATTR_ITEM 0)
> (258 EXTENT_DATA 0)
>
> But if we have the following sequence, then there is definitely
> something wrong, normally some INODE_ITEM is missing, like:
> (257 INODE_ITEM 0)
> (257 DIR_INDEX xxx)
> (257 DIR_ITEM xxx)
> (258 XATTR_ITEM 0) <<< objecitd suddenly changed to 258
> (258 EXTENT_DATA 0)
>
> So just by checking the previous key for above inode based key types, we
> can detect missing inode item.
>
> For INODE_REF key type, the check will be added along with INODE_REF
> checker.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/tree-checker.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index ccd5706199d7..636ce1b4566e 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -141,6 +141,19 @@ static int check_extent_data_item(struct extent_buffer *leaf,
> return -EUCLEAN;
> }
>
> + /*
> + * Previous key must have the same key->objectid (ino).
> + * It can be XATTR_ITEM, INODE_ITEM or just another EXTENT_DATA.
> + * But if objectids mismatch, it means we have a missing
> + * INODE_ITEM.
> + */
> + if (slot > 0 && prev_key->objectid != key->objectid) {
> + file_extent_err(leaf, slot,
> + "invalid previous key objectid, have %llu expect %llu",
> + prev_key->objectid, key->objectid);
> + return -EUCLEAN;
> + }
> +
> fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
>
> if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
> @@ -299,13 +312,21 @@ static void dir_item_err(const struct extent_buffer *eb, int slot,
> }
>
> static int check_dir_item(struct extent_buffer *leaf,
> - struct btrfs_key *key, int slot)
> + struct btrfs_key *key, struct btrfs_key *prev_key,
> + int slot)
> {
> struct btrfs_fs_info *fs_info = leaf->fs_info;
> struct btrfs_dir_item *di;
> u32 item_size = btrfs_item_size_nr(leaf, slot);
> u32 cur = 0;
>
> + /* Same check as in check_extent_data_item() */
> + if (slot > 0 && prev_key->objectid != key->objectid) {
> + dir_item_err(leaf, slot,
> + "invalid previous key objectid, have %llu expect %llu",
> + prev_key->objectid, key->objectid);
> + return -EUCLEAN;
> + }
> di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
> while (cur < item_size) {
> u32 name_len;
> @@ -841,7 +862,7 @@ static int check_leaf_item(struct extent_buffer *leaf,
> case BTRFS_DIR_ITEM_KEY:
> case BTRFS_DIR_INDEX_KEY:
> case BTRFS_XATTR_ITEM_KEY:
> - ret = check_dir_item(leaf, key, slot);
> + ret = check_dir_item(leaf, key, prev_key, slot);
> break;
> case BTRFS_BLOCK_GROUP_ITEM_KEY:
> ret = check_block_group_item(leaf, key, slot);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF
2019-08-26 11:45 ` Nikolay Borisov
@ 2019-08-26 11:50 ` Qu Wenruo
2019-09-23 15:45 ` David Sterba
0 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2019-08-26 11:50 UTC (permalink / raw)
To: Nikolay Borisov, Qu Wenruo, linux-btrfs
On 2019/8/26 下午7:45, Nikolay Borisov wrote:
>
>
> On 26.08.19 г. 10:40 ч., Qu Wenruo wrote:
>> For INODE_REF we will check:
>> - Objectid (ino) against previous key
>> To detect missing INODE_ITEM.
>>
>> - No overflow/padding in the data payload
>> Much like DIR_ITEM, but with less members to check.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/tree-checker.c | 53 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index 636ce1b4566e..3ce447eb591c 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -842,6 +842,56 @@ static int check_inode_item(struct extent_buffer *leaf,
>> return 0;
>> }
>>
>> +#define inode_ref_err(fs_info, eb, slot, fmt, ...) \
>> + inode_item_err(fs_info, eb, slot, fmt, __VA_ARGS__)
>
> This define doesn't bring anything, just opencode the call to
> inode_item_err directly.
I could argue we that in an inode ref context, using a inode_item_err()
doesn't look right.
And since it's doesn't do any hurt, I prefer to make the error message
parse to match the context.
Thanks,
Qu
>
>> +static int check_inode_ref(struct extent_buffer *leaf,
>> + struct btrfs_key *key, struct btrfs_key *prev_key,
>> + int slot)
>> +{
>> + struct btrfs_inode_ref *iref;
>> + unsigned long ptr;
>> + unsigned long end;
>> +
>> + /* namelen can't be 0, so item_size == sizeof() is also invalid */
>> + if (btrfs_item_size_nr(leaf, slot) <= sizeof(*iref)) {
>> + inode_ref_err(fs_info, leaf, slot,
>> + "invalid item size, have %u expect (%zu, %u)",
>> + btrfs_item_size_nr(leaf, slot),
>> + sizeof(*iref), BTRFS_LEAF_DATA_SIZE(leaf->fs_info));
>> + return -EUCLEAN;
>> + }
>> +
>> + ptr = btrfs_item_ptr_offset(leaf, slot);
>> + end = ptr + btrfs_item_size_nr(leaf, slot);
>> + while (ptr < end) {
>> + u16 namelen;
>> +
>> + if (ptr + sizeof(iref) > end) {
>> + inode_ref_err(fs_info, leaf, slot,
>> + "inode ref overflow, ptr %lu end %lu inode_ref_size %zu",
>> + ptr, end, sizeof(iref));
>> + return -EUCLEAN;
>> + }
>> +
>> + iref = (struct btrfs_inode_ref *)ptr;
>> + namelen = btrfs_inode_ref_name_len(leaf, iref);
>> + if (ptr + sizeof(*iref) + namelen > end) {
>> + inode_ref_err(fs_info, leaf, slot,
>> + "inode ref overflow, ptr %lu end %lu namelen %u",
>> + ptr, end, namelen);
>> + return -EUCLEAN;
>> + }
>> +
>> + /*
>> + * NOTE: In theory we should record all found index number
>> + * to find any duplicated index. But that will be too time
>> + * consuming for inodes with too many hard links.
>> + */
>> + ptr += sizeof(*iref) + namelen;
>> + }
>> + return 0;
>> +}
>> +
>> /*
>> * Common point to switch the item-specific validation.
>> */
>> @@ -864,6 +914,9 @@ static int check_leaf_item(struct extent_buffer *leaf,
>> case BTRFS_XATTR_ITEM_KEY:
>> ret = check_dir_item(leaf, key, prev_key, slot);
>> break;
>> + case BTRFS_INODE_REF_KEY:
>> + ret = check_inode_ref(leaf, key, prev_key, slot);
>> + break;
>> case BTRFS_BLOCK_GROUP_ITEM_KEY:
>> ret = check_block_group_item(leaf, key, slot);
>> break;
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF
2019-08-26 11:50 ` Qu Wenruo
@ 2019-09-23 15:45 ` David Sterba
0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-09-23 15:45 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Nikolay Borisov, Qu Wenruo, linux-btrfs
On Mon, Aug 26, 2019 at 07:50:03PM +0800, Qu Wenruo wrote:
>
>
> On 2019/8/26 下午7:45, Nikolay Borisov wrote:
> >
> >
> > On 26.08.19 г. 10:40 ч., Qu Wenruo wrote:
> >> For INODE_REF we will check:
> >> - Objectid (ino) against previous key
> >> To detect missing INODE_ITEM.
> >>
> >> - No overflow/padding in the data payload
> >> Much like DIR_ITEM, but with less members to check.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >> fs/btrfs/tree-checker.c | 53 +++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 53 insertions(+)
> >>
> >> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> >> index 636ce1b4566e..3ce447eb591c 100644
> >> --- a/fs/btrfs/tree-checker.c
> >> +++ b/fs/btrfs/tree-checker.c
> >> @@ -842,6 +842,56 @@ static int check_inode_item(struct extent_buffer *leaf,
> >> return 0;
> >> }
> >>
> >> +#define inode_ref_err(fs_info, eb, slot, fmt, ...) \
> >> + inode_item_err(fs_info, eb, slot, fmt, __VA_ARGS__)
> >
> > This define doesn't bring anything, just opencode the call to
> > inode_item_err directly.
>
> I could argue we that in an inode ref context, using a inode_item_err()
> doesn't look right.
>
> And since it's doesn't do any hurt, I prefer to make the error message
> parse to match the context.
I agree the alias inode_ref_err does not hurt, there's no penatly in the
code so for sake of readability let's do it.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF
2019-08-26 7:40 ` [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF Qu Wenruo
2019-08-26 11:45 ` Nikolay Borisov
@ 2019-09-23 15:47 ` David Sterba
1 sibling, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-09-23 15:47 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, Aug 26, 2019 at 03:40:39PM +0800, Qu Wenruo wrote:
> For INODE_REF we will check:
> - Objectid (ino) against previous key
> To detect missing INODE_ITEM.
>
> - No overflow/padding in the data payload
> Much like DIR_ITEM, but with less members to check.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/tree-checker.c | 53 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 636ce1b4566e..3ce447eb591c 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -842,6 +842,56 @@ static int check_inode_item(struct extent_buffer *leaf,
> return 0;
> }
>
> +#define inode_ref_err(fs_info, eb, slot, fmt, ...) \
> + inode_item_err(fs_info, eb, slot, fmt, __VA_ARGS__)
I've changed that to
#define inode_ref_err(fs_info, eb, slot, fmt, args...) \
inode_item_err(fs_info, eb, slot, fmt, ##args)
as this is the common style for the variable macro args used in btrfs.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-23 15:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26 7:40 [PATCH 0/2] btrfs: tree-checker: Add checks to detect missing INODE_ITEM Qu Wenruo
2019-08-26 7:40 ` [PATCH 1/2] btrfs: tree-checker: Try " Qu Wenruo
2019-08-26 11:46 ` Nikolay Borisov
2019-08-26 7:40 ` [PATCH 2/2] btrfs: tree-checker: Add check for INODE_REF Qu Wenruo
2019-08-26 11:45 ` Nikolay Borisov
2019-08-26 11:50 ` Qu Wenruo
2019-09-23 15:45 ` David Sterba
2019-09-23 15:47 ` David Sterba
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.