| Message ID | 20240717064424.11090-1-heming.zhao@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] ocfs2: fix the la space leak when unmounting an ocfs2 volume | expand |
[resent because gmail webapp appended html content after plain text] On 2024/7/17 14:44, Heming Zhao wrote: > This bug has existed since the initial OCFS2 code. The code logic > in ocfs2_sync_local_to_main() is wrong, as it ignores the last > contiguous free bits, which causes an OCFS2 volume to lose the > LA window size space on each umount command. > > Signed-off-by: Heming Zhao <heming.zhao@suse.com> LGTM: Reviewed-by: Su Yue <glass.su@suse.com> One nitpick below. > --- > fs/ocfs2/localalloc.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c > index 5df34561c551..0c45b139d9b9 100644 > --- a/fs/ocfs2/localalloc.c > +++ b/fs/ocfs2/localalloc.c > @@ -1002,6 +1002,25 @@ 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 && (start <= left)) { IMO if (count) is enough. start can not be left + 1 as start is calculated from bit_off + 1 in the above loop. So start - count is safe.
On 7/17/24 21:54, Su Yue wrote: > [resent because gmail webapp appended html content after plain text] > > On 2024/7/17 14:44, Heming Zhao wrote: >> This bug has existed since the initial OCFS2 code. The code logic >> in ocfs2_sync_local_to_main() is wrong, as it ignores the last >> contiguous free bits, which causes an OCFS2 volume to lose the >> LA window size space on each umount command. >> >> Signed-off-by: Heming Zhao <heming.zhao@suse.com> > > LGTM: > Reviewed-by: Su Yue <glass.su@suse.com> > One nitpick below. > >> --- >> fs/ocfs2/localalloc.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c >> index 5df34561c551..0c45b139d9b9 100644 >> --- a/fs/ocfs2/localalloc.c >> +++ b/fs/ocfs2/localalloc.c >> @@ -1002,6 +1002,25 @@ 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 && (start <= left)) { > > IMO if (count) is enough. start can not be left + 1 as start is calculated from bit_off + 1 in the above loop. So start - count is safe. > You are right, thanks for the review. -Heming
diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c index 5df34561c551..0c45b139d9b9 100644 --- a/fs/ocfs2/localalloc.c +++ b/fs/ocfs2/localalloc.c @@ -1002,6 +1002,25 @@ 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 && (start <= left)) { + 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);
This bug has existed since the initial OCFS2 code. The code logic in ocfs2_sync_local_to_main() is wrong, as it ignores the last contiguous free bits, which causes an OCFS2 volume to lose the LA window size space on each umount command. Signed-off-by: Heming Zhao <heming.zhao@suse.com> --- fs/ocfs2/localalloc.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)