On 2018年07月23日 14:22, Qu Wenruo wrote:
> In find_free_extent() under checks: tag, we have the following code
> 
> ------
>               search_start = ALIGN(offset, fs_info->stripesize);
>               /* move on to the next group */
>               if (search_start + num_bytes >
>                   block_group->key.objectid + block_group->key.offset) {
>                       btrfs_add_free_space(block_group, offset, num_bytes);
>                       goto loop;
>               }
>               if (offset < search_start)
>                       btrfs_add_free_space(block_group, offset,
>                                            search_start - offset);
>               BUG_ON(offset > search_start);
> ------
> 
> However ALIGN() is rounding down, thus @search_start <= @offset.

Well, ALIGN() is rounding up, so this is completely wrong.

But still we have some conflicting BUG_ON() against ALIGN().

I'll update the patch and hopes no one found my stupid error.

Thanks,
Qu

> 
> That later (offset < search_start) check will never be true, but that
> BUG_ON(offset > search_start) can be triggered as long as our @offset is
> not aligned to @stripesize.
> 
> However we never has such report on this BUG_ON(), which means the
> search result @offset is always aligned to @fs_info->stripesize (which
> is also set to sector size at mkfs time), or it's handled by block group
> range check.
> 
> Anyway, remove such dead unalignment check.
> 
> Signed-off-by: Qu Wenruo <w...@suse.com>
> ---
>  fs/btrfs/extent-tree.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f18e86177067..48a2aec7dc59 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -7754,7 +7754,7 @@ static noinline int find_free_extent(struct 
> btrfs_fs_info *fs_info,
>                       goto loop;
>               }
>  checks:
> -             search_start = ALIGN(offset, fs_info->stripesize);
> +             search_start = round_down(offset, fs_info->stripesize);
>  
>               /* move on to the next group */
>               if (search_start + num_bytes >
> @@ -7763,11 +7763,6 @@ static noinline int find_free_extent(struct 
> btrfs_fs_info *fs_info,
>                       goto loop;
>               }
>  
> -             if (offset < search_start)
> -                     btrfs_add_free_space(block_group, offset,
> -                                          search_start - offset);
> -             BUG_ON(offset > search_start);
> -
>               ret = btrfs_add_reserved_bytes(block_group, ram_bytes,
>                               num_bytes, delalloc);
>               if (ret == -EAGAIN) {
> 

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to