Re: [PATCH v2] ocfs2: fixing global bitmap allocating failure for discontig type

[Joseph Qi <joseph.qi@linux.alibaba.com>]

Thanks, I'll add you as "Tested-by".

On 2025/4/2 18:43, Gautham Ananthakrishna wrote:
> This patch also works fine.
> 
> Thanks,
> Gautham.
> ________________________________
> From: Joseph Qi <joseph.qi@linux.alibaba.com>
> Sent: Wednesday, April 2, 2025 7:02 AM
> To: Heming Zhao <heming.zhao@suse.com>; Gautham Ananthakrishna <gautham.ananthakrishna@oracle.com>
> Cc: ocfs2-devel@lists.linux.dev <ocfs2-devel@lists.linux.dev>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v2] ocfs2: fixing global bitmap allocating failure for discontig type
> 
> 
> 
> On 2025/4/1 22:26, Heming Zhao wrote:
>> The commit 4eb7b93e0310 ("ocfs2: improve write IO performance when
>> fragmentation is high") introduced a regression. In the discontiguous
>> extent allocation case, ocfs2_cluster_group_search() is comparing with
>> the wrong target length, which causes allocation failure.
>>
>> Call stack:
>> ocfs2_mkdir()
>>  ocfs2_reserve_new_inode()
>>   ocfs2_reserve_suballoc_bits()
>>    ocfs2_block_group_alloc()
>>     ocfs2_block_group_alloc_discontig()
>>      __ocfs2_claim_clusters()
>>       ocfs2_claim_suballoc_bits()
>>        ocfs2_search_chain()
>>         ocfs2_cluster_group_search()
>>
>> Reported-by: Gautham Ananthakrishna <gautham.ananthakrishna@oracle.com>
>> Fixes: 4eb7b93e0310 ("ocfs2: improve write IO performance when fragmentation is high")
>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> 
> Looks fine.
> Gautham, could you please verify this version? Thanks.
> 
> Joseph
> 
>> ---
>> v1 -> v2: OCFS2_AC_USE_MAIN_DISCONTIG type requires separate handling,
>>           with all other types proceeding as before.
>> ---
>>  fs/ocfs2/suballoc.c | 10 ++++++++--
>>  fs/ocfs2/suballoc.h |  1 +
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
>> index f7b483f0de2a..fde75f2af37a 100644
>> --- a/fs/ocfs2/suballoc.c
>> +++ b/fs/ocfs2/suballoc.c
>> @@ -698,10 +698,12 @@ static int ocfs2_block_group_alloc(struct ocfs2_super *osb,
>>
>>        bg_bh = ocfs2_block_group_alloc_contig(osb, handle, alloc_inode,
>>                                               ac, cl);
>> -     if (PTR_ERR(bg_bh) == -ENOSPC)
>> +     if (PTR_ERR(bg_bh) == -ENOSPC) {
>> +             ac->ac_which = OCFS2_AC_USE_MAIN_DISCONTIG;
>>                bg_bh = ocfs2_block_group_alloc_discontig(handle,
>>                                                          alloc_inode,
>>                                                          ac, cl);
>> +     }
>>        if (IS_ERR(bg_bh)) {
>>                status = PTR_ERR(bg_bh);
>>                bg_bh = NULL;
>> @@ -2365,7 +2367,8 @@ int __ocfs2_claim_clusters(handle_t *handle,
>>        BUG_ON(ac->ac_bits_given >= ac->ac_bits_wanted);
>>
>>        BUG_ON(ac->ac_which != OCFS2_AC_USE_LOCAL
>> -            && ac->ac_which != OCFS2_AC_USE_MAIN);
>> +            && ac->ac_which != OCFS2_AC_USE_MAIN
>> +            && ac->ac_which != OCFS2_AC_USE_MAIN_DISCONTIG);
>>
>>        if (ac->ac_which == OCFS2_AC_USE_LOCAL) {
>>                WARN_ON(min_clusters > 1);
>> @@ -2429,6 +2432,9 @@ int ocfs2_claim_clusters(handle_t *handle,
>>  {
>>        unsigned int bits_wanted = ac->ac_bits_wanted - ac->ac_bits_given;
>>
>> +     if (ac->ac_which == OCFS2_AC_USE_MAIN_DISCONTIG)
>> +             bits_wanted = min_clusters;
>> +
>>        return __ocfs2_claim_clusters(handle, ac, min_clusters,
>>                                      bits_wanted, cluster_start, num_clusters);
>>  }
>> diff --git a/fs/ocfs2/suballoc.h b/fs/ocfs2/suballoc.h
>> index b481b834857d..bcf2ed4a8631 100644
>> --- a/fs/ocfs2/suballoc.h
>> +++ b/fs/ocfs2/suballoc.h
>> @@ -29,6 +29,7 @@ struct ocfs2_alloc_context {
>>  #define OCFS2_AC_USE_MAIN  2
>>  #define OCFS2_AC_USE_INODE 3
>>  #define OCFS2_AC_USE_META  4
>> +#define OCFS2_AC_USE_MAIN_DISCONTIG  5
>>        u32    ac_which;
>>
>>        /* these are used by the chain search */
> 
>