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

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

[Guangshuo Li]

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@gmail.com>
---
 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;
-- 
2.43.0

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

[Joseph Qi]

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@gmail.com>
> ---
>  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

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

[Guangshuo Li]

On Thu, 9 Jul 2026 at 11:59, Joseph Qi <joseph.qi@linux.alibaba.com> 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@gmail.com>
> > ---
> >  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