[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <590fe587-0df7-4aa4-a8c0-b9120d658f6f@suse.com>
Date: Tue, 1 Apr 2025 21:50:10 +0800
From: Heming Zhao <heming.zhao@...e.com>
To: Joseph Qi <joseph.qi@...ux.alibaba.com>
Cc: ocfs2-devel@...ts.linux.dev, linux-kernel@...r.kernel.org,
gautham.ananthakrishna@...cle.com
Subject: Re: [PATCH] ocfs2: fixing global bitmap allocating failure for
discontig type
On 4/1/25 19:38, Joseph Qi wrote:
>
>
> On 2025/3/27 14:22, 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@...cle.com>
>> Fixes: 4eb7b93e0310 ("ocfs2: improve write IO performance when fragmentation is high")
>> Signed-off-by: Heming Zhao <heming.zhao@...e.com>
>> ---
>> fs/ocfs2/suballoc.c | 14 +++++++++++---
>> fs/ocfs2/suballoc.h | 1 +
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
>> index f7b483f0de2a..3dea082f6e91 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);
>> @@ -2427,7 +2430,12 @@ int ocfs2_claim_clusters(handle_t *handle,
>> u32 *cluster_start,
>> u32 *num_clusters)
>> {
>> - unsigned int bits_wanted = ac->ac_bits_wanted - ac->ac_bits_given;
>> + unsigned int bits_wanted;
>> +
>> + if (ac->ac_which == OCFS2_AC_USE_MAIN)
>> + bits_wanted = ac->ac_bits_wanted - ac->ac_bits_given;
>> + else /* ac_which == OCFS2_AC_USE_MAIN_DISCONTIG */
>> + bits_wanted = min_clusters;
>
> This looks wried. Why can not be other alloc type?
>
> Or it seems you intend to:
>
> 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;
>
> Thanks,
> Joseph
Yes, you are right.
We should use OCFS2_AC_USE_MAIN_DISCONTIG to handle the special discontig case.
Will send v2 patch for this mistake.
- Heming
>
>>
>> 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 */
>
Powered by blists - more mailing lists