| Message ID | 20250401142623.31223-1-heming.zhao@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ocfs2: fixing global bitmap allocating failure for discontig type | expand |
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 */
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 */ > >
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> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> Tested-by: Gautham Ananthakrishna <gautham.ananthakrishna@oracle.com> > --- > 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 */
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 */
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> --- 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(-)