On 12/20/2016 06:57 PM, Qu Wenruo wrote:
>
>
> At 12/20/2016 08:08 PM, Goldwyn Rodrigues wrote:
>> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
>>
>> root->highest_inode is not accurate at the time of creating a lost+found
>> and it fails because the highest_inode+1 is already present. This
>> could be
>> because of fixes after highest_inode is set. Instead, search
>> for the highest inode in the tree and use it for lost+found.
>>
>> This makes root->highest_inode unnecessary and hence deleted.
>
> This is much better than recording it in root.
>
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
>> ---
>> cmds-check.c | 46 +++++++++++++++++++++++++++-------------------
>> ctree.h | 1 -
>> disk-io.c | 1 -
>> 3 files changed, 27 insertions(+), 21 deletions(-)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index 1dba298..a55d00d 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -2853,6 +2853,31 @@ out:
>> return ret;
>> }
>>
>> +static int get_highest_inode(struct btrfs_trans_handle *trans,
>> + struct btrfs_root *root,
>> + struct btrfs_path *path,
>> + u64 *highest_ino)
>> +{
>> + struct btrfs_key key, found_key;
>> + int ret;
>> +
>> + btrfs_init_path(path);
>> + key.objectid = BTRFS_LAST_FREE_OBJECTID;
>> + key.offset = -1;
>> + key.type = BTRFS_INODE_ITEM_KEY;
>> + ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
>> + if (ret == 1) {
>> + btrfs_item_key_to_cpu(path->nodes[0], &found_key,
>> + path->slots[0] - 1);
>> + *highest_ino = found_key.objectid;
>> + ret = 0;
>> + }
>
> I think such search may cause problem.
>
> If the fs uses inode_map mount option, each fs tree will have a tailing
> FREE_INO and FREE_SPACE items.
>
> And FREE_INO/FREE_SPACE are all over LAST_FREE_OBJECTID.
>
> item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160
> inode generation 3 transid 7 size 0 nbytes 16384
> block group 0 mode 40755 links 1 uid 0 gid 0 rdev 0
> sequence 0 flags 0x1(none)
> item 1 key (256 INODE_REF 256) itemoff 16111 itemsize 12
> inode ref index 0 namelen 2 name: ..
> item 2 key (FREE_INO INODE_ITEM 0) itemoff 15951 itemsize 160
> inode generation 0 transid 7 size 0 nbytes 0
> block group 0 mode 100600 links 1 uid 0 gid 0 rdev 0
> sequence 24 flags 0x0(NOCOMPRESS|PREALLOC)
> item 3 key (FREE_SPACE UNTYPED 0) itemoff 15910 itemsize 41
> location key (FREE_INO INODE_ITEM 0)
> cache generation 0 entries 0 bitmaps 0
>
>
> In that case, such search will point to the FREE_INO slot, and always
> return -EOVERFLOW.
>
> What about check the objectid and if it's larger than
> LAST_FREE_OBJECTID, try to search previous slot?
If we are starting from LAST_FREE_OBJECTID which is -256ULL and smaller
than FREE_INO (-12ULL). -256ULL < -12ULL.
Won't a search for a (slot - 1) result in something smaller than
-256ULL? IOW, if it results in -12ULL then it is not a valid inode
anyways and hence should return -EOVERFLOW anyways.
>
> Other part looks good for me.
>
> Thanks,
> Qu
>> + if (*highest_ino >= BTRFS_LAST_FREE_OBJECTID)
>> + ret = -EOVERFLOW;
>> + btrfs_release_path(path);
>> + return ret;
>> +}
>> +
>> static int repair_inode_nlinks(struct btrfs_trans_handle *trans,
>> struct btrfs_root *root,
>> struct btrfs_path *path,
>> @@ -2898,11 +2923,9 @@ static int repair_inode_nlinks(struct
>> btrfs_trans_handle *trans,
>> }
>>
>> if (rec->found_link == 0) {
>> - lost_found_ino = root->highest_inode;
>> - if (lost_found_ino >= BTRFS_LAST_FREE_OBJECTID) {
>> - ret = -EOVERFLOW;
>> + ret = get_highest_inode(trans, root, path, &lost_found_ino);
>> + if (ret < 0)
>> goto out;
>> - }
>> lost_found_ino++;
>> ret = btrfs_mkdir(trans, root, dir_name, strlen(dir_name),
>> BTRFS_FIRST_FREE_OBJECTID, &lost_found_ino,
>> @@ -3266,21 +3289,6 @@ static int check_inode_recs(struct btrfs_root
>> *root,
>> }
>>
>> /*
>> - * We need to record the highest inode number for later 'lost+found'
>> - * dir creation.
>> - * We must select an ino not used/referred by any existing inode, or
>> - * 'lost+found' ino may be a missing ino in a corrupted leaf,
>> - * this may cause 'lost+found' dir has wrong nlinks.
>> - */
>> - cache = last_cache_extent(inode_cache);
>> - if (cache) {
>> - node = container_of(cache, struct ptr_node, cache);
>> - rec = node->data;
>> - if (rec->ino > root->highest_inode)
>> - root->highest_inode = rec->ino;
>> - }
>> -
>> - /*
>> * We need to repair backrefs first because we could change some
>> of the
>> * errors in the inode recs.
>> *
>> diff --git a/ctree.h b/ctree.h
>> index dd02ef8..0c34ae2 100644
>> --- a/ctree.h
>> +++ b/ctree.h
>> @@ -1177,7 +1177,6 @@ struct btrfs_root {
>>
>>
>> u32 type;
>> - u64 highest_inode;
>> u64 last_inode_alloc;
>>
>> /*
>> diff --git a/disk-io.c b/disk-io.c
>> index 9140a81..2a94d4f 100644
>> --- a/disk-io.c
>> +++ b/disk-io.c
>> @@ -494,7 +494,6 @@ void btrfs_setup_root(u32 nodesize, u32 leafsize,
>> u32 sectorsize,
>> root->fs_info = fs_info;
>> root->objectid = objectid;
>> root->last_trans = 0;
>> - root->highest_inode = 0;
>> root->last_inode_alloc = 0;
>>
>> INIT_LIST_HEAD(&root->dirty_list);
>>
>
>
>
--
Goldwyn
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
