[PATCH v2 0/2] Revert ocfs2 commit dfe6c5692fb5 and provide a new fix

[PATCH v2 0/2] Revert ocfs2 commit dfe6c5692fb5 and provide a new fix

[Heming Zhao]

SUSE QA team detected a mistake in my commit dfe6c5692fb5 ("ocfs2: fix
the la space leak when unmounting an ocfs2 volume"). I am very sorry for
my error. (If my eyes are correct) From the mailling list mails, this
patch shouldn't be applied to 4.19 5.4 5.10 5.15 6.1 6.6, and these
branches should perform a revert operation.

Reason for revert:
In commit dfe6c5692fb5, I mistakenly wrote: "This bug has existed since
the initial OCFS2 code.". The statement is wrong. The correct
introduction commit is 30dd3478c3cd. IOW, if the branch doesn't include
30dd3478c3cd, dfe6c5692fb5 should also not be included.

Changes in v2:
- revise the both commit logs
- following Joseph Qi's suggestion, apply patch [2/2] using more concise code.

Heming Zhao (2):
  ocfs2: Revert "ocfs2: fix the la space leak when unmounting an ocfs2
    volume"
  ocfs2: fix the space leak in LA when releasing LA

 fs/ocfs2/localalloc.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] ocfs2: Revert "ocfs2: fix the la space leak when unmounting an ocfs2 volume"

[Heming Zhao]

This reverts commit dfe6c5692fb5 ("ocfs2: fix the la space leak when
unmounting an ocfs2 volume").

In commit dfe6c5692fb5, the commit log "This bug has existed since the
initial OCFS2 code." is wrong. The correct introduction commit is
30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()").

The influence of commit dfe6c5692fb5 is that it provides a correct
fix for the latest kernel. however, it shouldn't be pushed to stable
branches. Let's use this commit to revert all branches that include
dfe6c5692fb5 and use a new fix method to fix commit 30dd3478c3cd.

Fixes: dfe6c5692fb5 ("ocfs2: fix the la space leak when unmounting an ocfs2 volume")
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
Cc: <stable@vger.kernel.org>
---
 fs/ocfs2/localalloc.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index 8ac42ea81a17..5df34561c551 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -1002,25 +1002,6 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
 		start = bit_off + 1;
 	}
 
-	/* clear the contiguous bits until the end boundary */
-	if (count) {
-		blkno = la_start_blk +
-			ocfs2_clusters_to_blocks(osb->sb,
-					start - count);
-
-		trace_ocfs2_sync_local_to_main_free(
-				count, start - count,
-				(unsigned long long)la_start_blk,
-				(unsigned long long)blkno);
-
-		status = ocfs2_release_clusters(handle,
-				main_bm_inode,
-				main_bm_bh, blkno,
-				count);
-		if (status < 0)
-			mlog_errno(status);
-	}
-
 bail:
 	if (status)
 		mlog_errno(status);
-- 
2.43.0

Re: [PATCH v2 1/2] ocfs2: Revert "ocfs2: fix the la space leak when unmounting an ocfs2 volume"

[Joseph Qi]

On 12/5/24 6:48 PM, Heming Zhao wrote:
> This reverts commit dfe6c5692fb5 ("ocfs2: fix the la space leak when
> unmounting an ocfs2 volume").
> 
> In commit dfe6c5692fb5, the commit log "This bug has existed since the
> initial OCFS2 code." is wrong. The correct introduction commit is
> 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()").
> 
> The influence of commit dfe6c5692fb5 is that it provides a correct
> fix for the latest kernel. however, it shouldn't be pushed to stable
> branches. Let's use this commit to revert all branches that include
> dfe6c5692fb5 and use a new fix method to fix commit 30dd3478c3cd.
> 
> Fixes: dfe6c5692fb5 ("ocfs2: fix the la space leak when unmounting an ocfs2 volume")
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> Cc: <stable@vger.kernel.org>
> ---
>  fs/ocfs2/localalloc.c | 19 -------------------
>  1 file changed, 19 deletions(-)
> 
> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
> index 8ac42ea81a17..5df34561c551 100644
> --- a/fs/ocfs2/localalloc.c
> +++ b/fs/ocfs2/localalloc.c
> @@ -1002,25 +1002,6 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>  		start = bit_off + 1;
>  	}
>  
> -	/* clear the contiguous bits until the end boundary */
> -	if (count) {
> -		blkno = la_start_blk +
> -			ocfs2_clusters_to_blocks(osb->sb,
> -					start - count);
> -
> -		trace_ocfs2_sync_local_to_main_free(
> -				count, start - count,
> -				(unsigned long long)la_start_blk,
> -				(unsigned long long)blkno);
> -
> -		status = ocfs2_release_clusters(handle,
> -				main_bm_inode,
> -				main_bm_bh, blkno,
> -				count);
> -		if (status < 0)
> -			mlog_errno(status);
> -	}
> -
>  bail:
>  	if (status)
>  		mlog_errno(status);

[PATCH v2 2/2] ocfs2: fix the space leak in LA when releasing LA

[Heming Zhao]

Commit 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()")
introduced an issue, the ocfs2_sync_local_to_main() ignores the last
contiguous free bits, which causes an OCFS2 volume to lose the last free
clusters of LA window during the release routine.

Please note, because commit dfe6c5692fb5 ("ocfs2: fix the la space leak
when unmounting an ocfs2 volume") was reverted, this commit is a
replacement fix for commit dfe6c5692fb5.

Fixes: 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()")
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/localalloc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index 5df34561c551..d1aa04a5af1b 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -971,9 +971,9 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
 	start = count = 0;
 	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
 
-	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start)) <
-	       left) {
-		if (bit_off == start) {
+	while (1) {
+		bit_off = ocfs2_find_next_zero_bit(bitmap, left, start);
+		if ((bit_off < left) && (bit_off == start)) {
 			count++;
 			start++;
 			continue;
@@ -998,6 +998,8 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
 			}
 		}
 
+		if (bit_off >= left)
+			break;
 		count = 1;
 		start = bit_off + 1;
 	}
-- 
2.43.0

Re: [PATCH v2 2/2] ocfs2: fix the space leak in LA when releasing LA

[Joseph Qi]

On 12/5/24 6:48 PM, Heming Zhao wrote:
> Commit 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()")
> introduced an issue, the ocfs2_sync_local_to_main() ignores the last
> contiguous free bits, which causes an OCFS2 volume to lose the last free
> clusters of LA window during the release routine.
> 
> Please note, because commit dfe6c5692fb5 ("ocfs2: fix the la space leak
> when unmounting an ocfs2 volume") was reverted, this commit is a
> replacement fix for commit dfe6c5692fb5.
> 
> Fixes: 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()")
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/localalloc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
> index 5df34561c551..d1aa04a5af1b 100644
> --- a/fs/ocfs2/localalloc.c
> +++ b/fs/ocfs2/localalloc.c
> @@ -971,9 +971,9 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>  	start = count = 0;
>  	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
>  
> -	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start)) <
> -	       left) {
> -		if (bit_off == start) {
> +	while (1) {
> +		bit_off = ocfs2_find_next_zero_bit(bitmap, left, start);
> +		if ((bit_off < left) && (bit_off == start)) {
>  			count++;
>  			start++;
>  			continue;
> @@ -998,6 +998,8 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>  			}
>  		}
>  
> +		if (bit_off >= left)
> +			break;
>  		count = 1;
>  		start = bit_off + 1;
>  	}

Re: [PATCH v2 2/2] ocfs2: fix the space leak in LA when releasing LA

[Joseph Qi]

On 12/5/24 7:10 PM, Joseph Qi wrote:
> 
> 
> On 12/5/24 6:48 PM, Heming Zhao wrote:
>> Commit 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()")
>> introduced an issue, the ocfs2_sync_local_to_main() ignores the last
>> contiguous free bits, which causes an OCFS2 volume to lose the last free
>> clusters of LA window during the release routine.
>>
>> Please note, because commit dfe6c5692fb5 ("ocfs2: fix the la space leak
>> when unmounting an ocfs2 volume") was reverted, this commit is a
>> replacement fix for commit dfe6c5692fb5.
>>
>> Fixes: 30dd3478c3cd ("ocfs2: correctly use ocfs2_find_next_zero_bit()")
>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
>> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> 
> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

Cc: <stable@vger.kernel.org> # 6.10+
>> ---
>>  fs/ocfs2/localalloc.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
>> index 5df34561c551..d1aa04a5af1b 100644
>> --- a/fs/ocfs2/localalloc.c
>> +++ b/fs/ocfs2/localalloc.c
>> @@ -971,9 +971,9 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>>  	start = count = 0;
>>  	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
>>  
>> -	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start)) <
>> -	       left) {
>> -		if (bit_off == start) {
>> +	while (1) {
>> +		bit_off = ocfs2_find_next_zero_bit(bitmap, left, start);
>> +		if ((bit_off < left) && (bit_off == start)) {
>>  			count++;
>>  			start++;
>>  			continue;
>> @@ -998,6 +998,8 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>>  			}
>>  		}
>>  
>> +		if (bit_off >= left)
>> +			break;
>>  		count = 1;
>>  		start = bit_off + 1;
>>  	}
>