Re: [POC for v5.15 0/2] btrfs: defrag: what if v5.15 is doing proper defrag

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/2/2 05:18, François-Xavier Thomas wrote:
>> Sure, I'll take the last 5.15 I used (5.15.13) as a base + the 2
>> patches from this thread, is that good?
>>     1-btrfs-dont-defrag-preallocated-extents.patch
>>     2-defrag-limit-cluster-size-to-first-hole.patch
>>
>> Will post results tomorrow when it finishes compiling.
>
> Tomorrow ended up taking a week, as it happens, sorry for that. No
> significant difference compared to before:
> https://i.imgur.com/BNoxixL.png
>
> I'm not sure what we can infer from this, if I understood correctly
> this should have shown similar amounts of I/O to 5.16 with all the
> previous fixes, right?

OK, things are getting more complex now.

Unless your workload includes quite some preallocated range, it's
definitely not what I expected.

As a final struggle, mind to test the small diff (without any previous
diff) against v5.15?

Thanks,
Qu

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index cc61813213d8..bcc075ea56c9 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1528,8 +1528,6 @@ int btrfs_defrag_file(struct inode *inode, struct
file *file,
                         cluster = (PAGE_ALIGN(defrag_end) >>
                                    PAGE_SHIFT) - i;
                         cluster = min(cluster, max_cluster);
-               } else {
-                       cluster = max_cluster;
                 }

                 if (i + cluster > ra_index) {


>
> I pasted the full diff I used at the end of this mail, since
> `defrag_lookup_extent` changed signature between 5.15 and 5.16 I had
> to do a couple of minor changes to get it to compile on 5.15.13.
> Hopefully that was what you initially intended on testing.
>
> Thanks,
> François-Xavier
>
> ----
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index cc61813213d8..07eb7417b6e7 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1119,19 +1119,23 @@ static struct extent_map
> *defrag_lookup_extent(struct inode *inode, u64 start)
>   static bool defrag_check_next_extent(struct inode *inode, struct
> extent_map *em)
>   {
>       struct extent_map *next;
> -    bool ret = true;
> +    bool ret = false;
>
>       /* this is the last extent */
>       if (em->start + em->len >= i_size_read(inode))
> -        return false;
> +        return ret;
>
>       next = defrag_lookup_extent(inode, em->start + em->len);
>       if (!next || next->block_start >= EXTENT_MAP_LAST_BYTE)
> -        ret = false;
> -    else if ((em->block_start + em->block_len == next->block_start) &&
> -         (em->block_len > SZ_128K && next->block_len > SZ_128K))
> -        ret = false;
> +        goto out;
> +    if (test_bit(EXTENT_FLAG_COMPRESSED, &next->flags))
> +        goto out;
> +    if ((em->block_start + em->block_len == next->block_start) &&
> +     (em->block_len > SZ_128K && next->block_len > SZ_128K))
> +        goto out;
>
> +    ret = true;
> +out:
>       free_extent_map(next);
>       return ret;
>   }
> @@ -1403,6 +1407,30 @@ static int cluster_pages_for_defrag(struct inode *inode,
>
>   }
>
> +static u64 get_cluster_size(struct inode *inode, u64 start, u64 len)
> +{
> +    u64 cur = start;
> +    u64 cluster_len = 0;
> +    while (cur < start + len) {
> +        struct extent_map *em;
> +
> +        em = defrag_lookup_extent(inode, cur);
> +        if (!em)
> +            break;
> +        /*
> +         * Here we don't do comprehensive checks, we just
> +         * find the first hole/preallocated.
> +         */
> +        if (em->block_start >= EXTENT_MAP_LAST_BYTE ||
> +            test_bit(EXTENT_FLAG_PREALLOC, &em->flags))
> +            break;
> +        cluster_len += min(em->start + em->len - cur, start + len - cur);
> +        cur = min(em->start + em->len, start + len);
> +    }
> +    return cluster_len;
> +}
> +
> +
>   int btrfs_defrag_file(struct inode *inode, struct file *file,
>                 struct btrfs_ioctl_defrag_range_args *range,
>                 u64 newer_than, unsigned long max_to_defrag)
> @@ -1531,6 +1559,13 @@ int btrfs_defrag_file(struct inode *inode,
> struct file *file,
>           } else {
>               cluster = max_cluster;
>           }
> +        cluster = min_t(unsigned long,
> +                get_cluster_size(inode, i << PAGE_SHIFT,
> +                         cluster << PAGE_SHIFT)
> +                    >> PAGE_SHIFT,
> +                    cluster);
> +        if (cluster == 0)
> +            goto next;
>
>           if (i + cluster > ra_index) {
>               ra_index = max(i, ra_index);
> @@ -1557,6 +1592,7 @@ int btrfs_defrag_file(struct inode *inode,
> struct file *file,
>           balance_dirty_pages_ratelimited(inode->i_mapping);
>           btrfs_inode_unlock(inode, 0);
>
> +next:
>           if (newer_than) {
>               if (newer_off == (u64)-1)
>                   break;