Re: [PATCH] ocfs2: free claimed clusters when defrag move fails

[Guangshuo Li]

On Thu, 9 Jul 2026 at 11:59, Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx> wrote:
>
>
>
> On 7/8/26 2:22 PM, Guangshuo Li wrote:
> > ocfs2_defrag_extent() claims new clusters before calling
> > __ocfs2_move_extent(). If __ocfs2_move_extent() fails, the newly claimed
> > clusters have not been attached to the inode extent tree, but the error
> > path only logs the error and continues.
> >
> > The following ocfs2_cow_sync_writeback() call can then overwrite the
> > original error with 0, while the claimed clusters are left allocated and
> > unreferenced.
> >
> > Set need_free and leave through out_commit when __ocfs2_move_extent()
> > fails, so the claimed clusters are released and the original error is
> > returned to the caller.
> >
> > Fixes: 202ee5facb2c ("Ocfs2/move_extents: defrag a range of extent.")
> > Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> > ---
> >  fs/ocfs2/move_extents.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
> > index ad1678ee7cc4..4f1745d58a88 100644
> > --- a/fs/ocfs2/move_extents.c
> > +++ b/fs/ocfs2/move_extents.c
> > @@ -310,8 +310,11 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
> >
> >       ret = __ocfs2_move_extent(handle, context, cpos, new_len, phys_cpos,
> >                                 new_phys_cpos, ext_flags);
> > -     if (ret)
> > +     if (ret) {
> >               mlog_errno(ret);
> > +             need_free = 1;
> > +             goto out_commit;
> > +     }
> >
> >       if (partial && (new_len != *len))
> >               *len = new_len;
>
> __ocfs2_move_extent() calls ocfs2_split_extent() partway through, which
> repoints the tree at new_phys_cpos, then can still fail in
> ocfs2_decrease_refcount() (-ENOMEM from ocfs2_read_refcount_block()) or
> ocfs2_truncate_log_append() (-ENOSPC). Neither aborts the handle.
>
> On those failures the new out_commit path frees new_phys_cpos/new_len and
> commits, so the clusters are both referenced by the extent tree and marked
> free.
>
> Thanks,
> Joseph

Thanks, you are right. My patch incorrectly assumed that every error
from __ocfs2_move_extent() occurs before the extent tree is repointed.
The ext_flags validation failure I was targeting is pre-split, but
ocfs2_decrease_refcount() and ocfs2_truncate_log_append() can fail
after a successful split. I will rework the fix to preserve the
original error while freeing the claimed clusters only when the split
has not been attempted.

Thanks，
Guangshuo