diff mbox series

[5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

Message ID 20190325082253.19583-6-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: check: Check and repair invalid free space cahce inode mode | expand

Commit Message

Qu Wenruo March 25, 2019, 8:22 a.m. UTC
Unlike inodes in fs roots, we don't really check the inode items in root
tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.

This makes invalid inode items sneak into root tree.
For example:
        item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
                generation 30 transid 30 size 65536 nbytes 1507328
                block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
				   ^ Should be 100600
                sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
                atime 0.0 (1970-01-01 08:00:00)
                ctime 1553491158.189771625 (2019-03-25 13:19:18)
                mtime 0.0 (1970-01-01 08:00:00)
                otime 0.0 (1970-01-01 08:00:00)

There is a report of such problem in the mail list.

This patch will check and repair inode items of free space cache inodes in
lowmem mode.

Since free space cache inodes doesn't have INODE_REF but still has 1
link, we can't use check_inode_item() directly.
Instead we only check the inode mode, as that's the important part.

The check and repair function: check_repair_free_space_inode() is also
exported for original mode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.h |  1 +
 check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 check/mode-lowmem.h |  2 ++
 3 files changed, 48 insertions(+)

Comments

Su Yue March 25, 2019, 2:36 p.m. UTC | #1
On 2019/3/25 4:22 PM, Qu Wenruo wrote:
> Unlike inodes in fs roots, we don't really check the inode items in root
> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
>
> This makes invalid inode items sneak into root tree.
> For example:
>          item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>                  generation 30 transid 30 size 65536 nbytes 1507328
>                  block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
> 				   ^ Should be 100600
>                  sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>                  atime 0.0 (1970-01-01 08:00:00)
>                  ctime 1553491158.189771625 (2019-03-25 13:19:18)
>                  mtime 0.0 (1970-01-01 08:00:00)
>                  otime 0.0 (1970-01-01 08:00:00)
>
> There is a report of such problem in the mail list.
>
> This patch will check and repair inode items of free space cache inodes in
> lowmem mode.
>
> Since free space cache inodes doesn't have INODE_REF but still has 1
> link, we can't use check_inode_item() directly.
> Instead we only check the inode mode, as that's the important part.
>
> The check and repair function: check_repair_free_space_inode() is also
> exported for original mode.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   check/mode-common.h |  1 +
>   check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   check/mode-lowmem.h |  2 ++
>   3 files changed, 48 insertions(+)
>
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 8895747adf1d..855f83617854 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -24,6 +24,7 @@
>   #include <sys/stat.h>
>   #include "ctree.h"
>
> +#define FREE_SPACE_CACHE_INODE_MODE	(0100600)
>   /*
>    * Use for tree walk to walk through trees whose leaves/nodes can be shared
>    * between different trees. (Namely subvolume/fs trees)
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index f37c3b42e6b6..cdb9b66a63a3 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -5091,6 +5091,40 @@ out:
>   	return err;
>   }
>
> +/*
> + * For free space inodes, we can't call check_inode_item() as free space
> + * cache inode doesn't have INODE_REF.
> + * We just check its inode mode.
> + */
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path)
> +{


In personal taste, I prefer check_free_space_inode() to
follow existed code.

> +	struct btrfs_inode_item *iitem;
> +	struct btrfs_key key;
> +	u32 mode;
> +	int ret = 0;
> +
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			       struct btrfs_inode_item);
> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
> +		error(
> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
> +		ret = -EUCLEAN;
> +		if (repair) {
> +			ret = repair_imode_lowmem(fs_info->tree_root,
> +						  path);
> +			if (ret < 0)
> +				return ret;
> +			return ret;
> +		}
> +	}
> +	return ret;
> +}
> +
>   /*
>    * Check all fs/file tree in low_memory mode.
>    *
> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>   		btrfs_item_key_to_cpu(node, &key, slot);
>   		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>   			goto out;
> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
> +		    is_fstree(key.objectid)) {
> +			ret = check_repair_free_space_inode(fs_info, &path);
> +			/* Check if we still have a valid path to continue */

'path.nodes[0]' is quite interesting :).
What about the situation btrfs_search_slot() succeed but failed to
commit a transaction?


---
Su
> +			if (ret < 0 && path.nodes[0]) {
> +				err |= ret;
> +				goto next;
> +			}
> +			if (ret < 0 && !path.nodes[0])
> +				goto out;
> +		}
>   		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>   		    fs_root_objectid(key.objectid)) {
>   			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
> index e0ab30b770d5..d2983fd12eb4 100644
> --- a/check/mode-lowmem.h
> +++ b/check/mode-lowmem.h
> @@ -67,5 +67,7 @@
>
>   int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>   int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path);
>
>   #endif
>
Qu Wenruo March 25, 2019, 2:39 p.m. UTC | #2
On 2019/3/25 下午10:36, Su Yue wrote:
> 
> 
> On 2019/3/25 4:22 PM, Qu Wenruo wrote:
>> Unlike inodes in fs roots, we don't really check the inode items in root
>> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
>>
>> This makes invalid inode items sneak into root tree.
>> For example:
>>          item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>>                  generation 30 transid 30 size 65536 nbytes 1507328
>>                  block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
>>                    ^ Should be 100600
>>                  sequence 23 flags
>> 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>>                  atime 0.0 (1970-01-01 08:00:00)
>>                  ctime 1553491158.189771625 (2019-03-25 13:19:18)
>>                  mtime 0.0 (1970-01-01 08:00:00)
>>                  otime 0.0 (1970-01-01 08:00:00)
>>
>> There is a report of such problem in the mail list.
>>
>> This patch will check and repair inode items of free space cache
>> inodes in
>> lowmem mode.
>>
>> Since free space cache inodes doesn't have INODE_REF but still has 1
>> link, we can't use check_inode_item() directly.
>> Instead we only check the inode mode, as that's the important part.
>>
>> The check and repair function: check_repair_free_space_inode() is also
>> exported for original mode.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   check/mode-common.h |  1 +
>>   check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   check/mode-lowmem.h |  2 ++
>>   3 files changed, 48 insertions(+)
>>
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 8895747adf1d..855f83617854 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -24,6 +24,7 @@
>>   #include <sys/stat.h>
>>   #include "ctree.h"
>>
>> +#define FREE_SPACE_CACHE_INODE_MODE    (0100600)
>>   /*
>>    * Use for tree walk to walk through trees whose leaves/nodes can be
>> shared
>>    * between different trees. (Namely subvolume/fs trees)
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index f37c3b42e6b6..cdb9b66a63a3 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -5091,6 +5091,40 @@ out:
>>       return err;
>>   }
>>
>> +/*
>> + * For free space inodes, we can't call check_inode_item() as free space
>> + * cache inode doesn't have INODE_REF.
>> + * We just check its inode mode.
>> + */
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +                  struct btrfs_path *path)
>> +{
> 
> 
> In personal taste, I prefer check_free_space_inode() to
> follow existed code.
> 
>> +    struct btrfs_inode_item *iitem;
>> +    struct btrfs_key key;
>> +    u32 mode;
>> +    int ret = 0;
>> +
>> +    btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +    ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>> +    iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +                   struct btrfs_inode_item);
>> +    mode = btrfs_inode_mode(path->nodes[0], iitem);
>> +    if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>> +        error(
>> +    "free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>> +            key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>> +        ret = -EUCLEAN;
>> +        if (repair) {
>> +            ret = repair_imode_lowmem(fs_info->tree_root,
>> +                          path);
>> +            if (ret < 0)
>> +                return ret;
>> +            return ret;
>> +        }
>> +    }
>> +    return ret;
>> +}
>> +
>>   /*
>>    * Check all fs/file tree in low_memory mode.
>>    *
>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>> *fs_info)
>>           btrfs_item_key_to_cpu(node, &key, slot);
>>           if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>               goto out;
>> +        if (key.type == BTRFS_INODE_ITEM_KEY &&
>> +            is_fstree(key.objectid)) {
>> +            ret = check_repair_free_space_inode(fs_info, &path);
>> +            /* Check if we still have a valid path to continue */
> 
> 'path.nodes[0]' is quite interesting :).
> What about the situation btrfs_search_slot() succeed but failed to
> commit a transaction?

As long as path.nodes[0] exists, we still have a valid path of root
tree, check can continue going.

For that valid path but failed commit case, I believe the most possible
problem would be a corrupted extent tree. And in that case, we can still
continue checking fs tree.

Thanks,
Qu

> 
> 
> ---
> Su
>> +            if (ret < 0 && path.nodes[0]) {
>> +                err |= ret;
>> +                goto next;
>> +            }
>> +            if (ret < 0 && !path.nodes[0])
>> +                goto out;
>> +        }
>>           if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>               fs_root_objectid(key.objectid)) {
>>               if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>> index e0ab30b770d5..d2983fd12eb4 100644
>> --- a/check/mode-lowmem.h
>> +++ b/check/mode-lowmem.h
>> @@ -67,5 +67,7 @@
>>
>>   int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>   int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +                  struct btrfs_path *path);
>>
>>   #endif
>>
>
Nikolay Borisov March 29, 2019, 9:40 a.m. UTC | #3
On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
> Unlike inodes in fs roots, we don't really check the inode items in root
> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
> 
> This makes invalid inode items sneak into root tree.
> For example:
>         item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>                 generation 30 transid 30 size 65536 nbytes 1507328
>                 block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
> 				   ^ Should be 100600
>                 sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>                 atime 0.0 (1970-01-01 08:00:00)
>                 ctime 1553491158.189771625 (2019-03-25 13:19:18)
>                 mtime 0.0 (1970-01-01 08:00:00)
>                 otime 0.0 (1970-01-01 08:00:00)
> 
> There is a report of such problem in the mail list.
> 
> This patch will check and repair inode items of free space cache inodes in
> lowmem mode.
> 
> Since free space cache inodes doesn't have INODE_REF but still has 1
> link, we can't use check_inode_item() directly.
> Instead we only check the inode mode, as that's the important part.
> 
> The check and repair function: check_repair_free_space_inode() is also
> exported for original mode.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/mode-common.h |  1 +
>  check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  check/mode-lowmem.h |  2 ++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 8895747adf1d..855f83617854 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -24,6 +24,7 @@
>  #include <sys/stat.h>
>  #include "ctree.h"
>  
> +#define FREE_SPACE_CACHE_INODE_MODE	(0100600)
>  /*
>   * Use for tree walk to walk through trees whose leaves/nodes can be shared
>   * between different trees. (Namely subvolume/fs trees)
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index f37c3b42e6b6..cdb9b66a63a3 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -5091,6 +5091,40 @@ out:
>  	return err;
>  }
>  
> +/*
> + * For free space inodes, we can't call check_inode_item() as free space
> + * cache inode doesn't have INODE_REF.
> + * We just check its inode mode.
> + */
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path)
> +{
> +	struct btrfs_inode_item *iitem;
> +	struct btrfs_key key;
> +	u32 mode;
> +	int ret = 0;
> +
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			       struct btrfs_inode_item);
> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
> +		error(
> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
> +		ret = -EUCLEAN;
> +		if (repair) {
> +			ret = repair_imode_lowmem(fs_info->tree_root,
> +						  path);
> +			if (ret < 0)
> +				return ret;
> +			return ret;
> +		}
> +	}
> +	return ret;
> +}

You put this function in mode-lowmem, yet it's also used in the next
patch to implement check/repair for original mode. Since you are oging
to re-use function across the 2 modes it makes no sense to separate them
in their names. Perhaps put them in mode-common.c

> +
>  /*
>   * Check all fs/file tree in low_memory mode.
>   *
> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>  		btrfs_item_key_to_cpu(node, &key, slot);
>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>  			goto out;
> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
> +		    is_fstree(key.objectid)) {
> +			ret = check_repair_free_space_inode(fs_info, &path);
> +			/* Check if we still have a valid path to continue */
> +			if (ret < 0 && path.nodes[0]) {
> +				err |= ret;
> +				goto next;
> +			}
> +			if (ret < 0 && !path.nodes[0])
> +				goto out;
> +		}
>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>  		    fs_root_objectid(key.objectid)) {
>  			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
> index e0ab30b770d5..d2983fd12eb4 100644
> --- a/check/mode-lowmem.h
> +++ b/check/mode-lowmem.h
> @@ -67,5 +67,7 @@
>  
>  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>  int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path);
>  
>  #endif
>
Qu Wenruo March 29, 2019, 11:02 a.m. UTC | #4
[snip]
>> +/*
>> + * For free space inodes, we can't call check_inode_item() as free space
>> + * cache inode doesn't have INODE_REF.
>> + * We just check its inode mode.
>> + */
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +				  struct btrfs_path *path)
>> +{
>> +	struct btrfs_inode_item *iitem;
>> +	struct btrfs_key key;
>> +	u32 mode;
>> +	int ret = 0;
>> +
>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +			       struct btrfs_inode_item);
>> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
>> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>> +		error(
>> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>> +		ret = -EUCLEAN;
>> +		if (repair) {
>> +			ret = repair_imode_lowmem(fs_info->tree_root,
>> +						  path);
>> +			if (ret < 0)
>> +				return ret;
>> +			return ret;
>> +		}
>> +	}
>> +	return ret;
>> +}
> 
> You put this function in mode-lowmem, yet it's also used in the next
> patch to implement check/repair for original mode. Since you are oging
> to re-use function across the 2 modes it makes no sense to separate them
> in their names. Perhaps put them in mode-common.c
> 

The limitation is due to repair_imode_lowmem().

Thanks,
Qu

>> +
>>  /*
>>   * Check all fs/file tree in low_memory mode.
>>   *
>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>>  		btrfs_item_key_to_cpu(node, &key, slot);
>>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>  			goto out;
>> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
>> +		    is_fstree(key.objectid)) {
>> +			ret = check_repair_free_space_inode(fs_info, &path);
>> +			/* Check if we still have a valid path to continue */
>> +			if (ret < 0 && path.nodes[0]) {
>> +				err |= ret;
>> +				goto next;
>> +			}
>> +			if (ret < 0 && !path.nodes[0])
>> +				goto out;
>> +		}
>>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>  		    fs_root_objectid(key.objectid)) {
>>  			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>> index e0ab30b770d5..d2983fd12eb4 100644
>> --- a/check/mode-lowmem.h
>> +++ b/check/mode-lowmem.h
>> @@ -67,5 +67,7 @@
>>  
>>  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>  int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +				  struct btrfs_path *path);
>>  
>>  #endif
>>
Nikolay Borisov March 29, 2019, 11:47 a.m. UTC | #5
On 29.03.19 г. 13:02 ч., Qu Wenruo wrote:
> [snip]
>>> +/*
>>> + * For free space inodes, we can't call check_inode_item() as free space
>>> + * cache inode doesn't have INODE_REF.
>>> + * We just check its inode mode.
>>> + */
>>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>>> +				  struct btrfs_path *path)
>>> +{
>>> +	struct btrfs_inode_item *iitem;
>>> +	struct btrfs_key key;
>>> +	u32 mode;
>>> +	int ret = 0;
>>> +
>>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>>> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>>> +			       struct btrfs_inode_item);
>>> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
>>> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>>> +		error(
>>> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>>> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>>> +		ret = -EUCLEAN;
>>> +		if (repair) {
>>> +			ret = repair_imode_lowmem(fs_info->tree_root,
>>> +						  path);
>>> +			if (ret < 0)
>>> +				return ret;
>>> +			return ret;
>>> +		}
>>> +	}
>>> +	return ret;
>>> +}
>>
>> You put this function in mode-lowmem, yet it's also used in the next
>> patch to implement check/repair for original mode. Since you are oging
>> to re-use function across the 2 modes it makes no sense to separate them
>> in their names. Perhaps put them in mode-common.c
>>
> 
> The limitation is due to repair_imode_lowmem().

move it to the common file as well ?

> 
> Thanks,
> Qu
> 
>>> +
>>>  /*
>>>   * Check all fs/file tree in low_memory mode.
>>>   *
>>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>>>  		btrfs_item_key_to_cpu(node, &key, slot);
>>>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>>  			goto out;
>>> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
>>> +		    is_fstree(key.objectid)) {
>>> +			ret = check_repair_free_space_inode(fs_info, &path);
>>> +			/* Check if we still have a valid path to continue */
>>> +			if (ret < 0 && path.nodes[0]) {
>>> +				err |= ret;
>>> +				goto next;
>>> +			}
>>> +			if (ret < 0 && !path.nodes[0])
>>> +				goto out;
>>> +		}
>>>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>>  		    fs_root_objectid(key.objectid)) {
>>>  			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>>> index e0ab30b770d5..d2983fd12eb4 100644
>>> --- a/check/mode-lowmem.h
>>> +++ b/check/mode-lowmem.h
>>> @@ -67,5 +67,7 @@
>>>  
>>>  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>>  int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>>> +				  struct btrfs_path *path);
>>>  
>>>  #endif
>>>
diff mbox series

Patch

diff --git a/check/mode-common.h b/check/mode-common.h
index 8895747adf1d..855f83617854 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -24,6 +24,7 @@ 
 #include <sys/stat.h>
 #include "ctree.h"
 
+#define FREE_SPACE_CACHE_INODE_MODE	(0100600)
 /*
  * Use for tree walk to walk through trees whose leaves/nodes can be shared
  * between different trees. (Namely subvolume/fs trees)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index f37c3b42e6b6..cdb9b66a63a3 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -5091,6 +5091,40 @@  out:
 	return err;
 }
 
+/*
+ * For free space inodes, we can't call check_inode_item() as free space
+ * cache inode doesn't have INODE_REF.
+ * We just check its inode mode.
+ */
+int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
+				  struct btrfs_path *path)
+{
+	struct btrfs_inode_item *iitem;
+	struct btrfs_key key;
+	u32 mode;
+	int ret = 0;
+
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
+	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			       struct btrfs_inode_item);
+	mode = btrfs_inode_mode(path->nodes[0], iitem);
+	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
+		error(
+	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
+			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
+		ret = -EUCLEAN;
+		if (repair) {
+			ret = repair_imode_lowmem(fs_info->tree_root,
+						  path);
+			if (ret < 0)
+				return ret;
+			return ret;
+		}
+	}
+	return ret;
+}
+
 /*
  * Check all fs/file tree in low_memory mode.
  *
@@ -5130,6 +5164,17 @@  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
 		btrfs_item_key_to_cpu(node, &key, slot);
 		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
 			goto out;
+		if (key.type == BTRFS_INODE_ITEM_KEY &&
+		    is_fstree(key.objectid)) {
+			ret = check_repair_free_space_inode(fs_info, &path);
+			/* Check if we still have a valid path to continue */
+			if (ret < 0 && path.nodes[0]) {
+				err |= ret;
+				goto next;
+			}
+			if (ret < 0 && !path.nodes[0])
+				goto out;
+		}
 		if (key.type == BTRFS_ROOT_ITEM_KEY &&
 		    fs_root_objectid(key.objectid)) {
 			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
index e0ab30b770d5..d2983fd12eb4 100644
--- a/check/mode-lowmem.h
+++ b/check/mode-lowmem.h
@@ -67,5 +67,7 @@ 
 
 int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
 int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
+int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
+				  struct btrfs_path *path);
 
 #endif