Re: [PATCH] ocfs2: defer suballocator block group reclaim to workqueue

[Joseph Qi <joseph.qi@linux.alibaba.com>]

On 8/28/26 1:47 PM, Heming Zhao wrote:
> On Thu, Aug 27, 2026 at 08:43:13PM +0800, Joseph Qi wrote:
>> When the last bit in a suballocator block group is freed,
>> _ocfs2_free_suballoc_bits() reclaims the group back to the global
>> bitmap.  The reclaim takes inode_lock() on the global bitmap inode
>> while running inside the freeing transaction, adding a lock
>> dependency of
>>
>>   j_trans_barrier -> global bitmap inode i_rwsem
>>
>> This forms a circular dependency with paths such as
>> ocfs2_shutdown_local_alloc(), which take the global bitmap inode
>> lock before starting a transaction:
>>
>>   Task1 (dealloc):
>>     ocfs2_run_deallocs
>>       ocfs2_free_cached_blocks
>>         ocfs2_start_trans
>>           down_read(j_trans_barrier)
>>         _ocfs2_free_suballoc_bits
>>           _ocfs2_reclaim_suballoc_to_main
>>             inode_lock(main_bm_inode)  <- wait on Task2
>>
>>   Task2 (dismount):
>>     ocfs2_shutdown_local_alloc
>>       inode_lock(main_bm_inode)
>>       ocfs2_start_trans
>>         down_read(j_trans_barrier)  <- wait on Task3
>>
>>   Task3 (ocfs2cmt):
>>     ocfs2_commit_cache
>>       down_write(j_trans_barrier)  <- wait on Task1's handle
>>       jbd2_journal_flush
>>
>> Task1 waits for Task2's inode_lock(), Task2 waits for the
>> j_trans_barrier down_write() held by ocfs2cmt, and ocfs2cmt waits
>> for Task1's running transaction to commit - a real deadlock,
>> observed with aio-stress direct IO writes racing dismount.
>>
>> Fix it by deferring the reclaim to the per-superblock ocfs2_wq
>> workqueue, so the freeing transaction no longer takes the global
>> bitmap inode lock.  The worker re-checks under the suballocator
>> locks that the block group is still fully freed (it may have been
>> allocated from again in the meantime), takes the global bitmap
>> inode locks before starting its own transaction, and performs the
>> same suballocator cleanup and space return.  The inode lock order
>> (suballocator inode -> global bitmap inode) is consistent with the
>> existing "inode lock before transaction" order, breaking the cycle.
>>
>> Reclaim work can still be queued late in dismount, e.g. when orphan
>> dir recovery frees inode bits, so both ocfs2_dismount_volume() and
>> the mount error path flush ocfs2_wq right before the journal is
>> shut down to make sure no reclaim work is left running.  The worker
>> also bails out if the journal is already gone.
>>
>> Tested with the ocfs2 testsuite (including aio-stress direct IO)
>> and umount/mount cycles on a CONFIG_PROVE_LOCKING kernel: the
>> circular locking dependency is gone and freed block groups are
>> still returned to the global bitmap.
>>
>> Fixes: 4a54331616b3 ("ocfs2: give ocfs2 the ability to reclaim suballocator free bg")
>> Assisted-by: Qoder:Qwen3.8-Max
>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>> ---
>>  fs/ocfs2/ocfs2.h    |   5 ++
>>  fs/ocfs2/suballoc.c | 202 +++++++++++++++++++++++++++++++++++++-------
>>  fs/ocfs2/suballoc.h |   2 +-
>>  fs/ocfs2/super.c    |  12 +++
>>  4 files changed, 191 insertions(+), 30 deletions(-)
>>

......

>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
>> index c62e389d4dd6..c1aafbb9500a 100644
>> --- a/fs/ocfs2/super.c
>> +++ b/fs/ocfs2/super.c
>> @@ -1779,6 +1779,9 @@ static int ocfs2_mount_volume(struct super_block *sb)
>>  	if (osb->local_alloc_state == OCFS2_LA_ENABLED)
>>  		ocfs2_shutdown_local_alloc(osb);
>>  	ocfs2_release_system_inodes(osb);
>> +	/* Drain pending suballoc reclaim work before the journal goes away */
>> +	if (osb->ocfs2_wq)
>> +		flush_workqueue(osb->ocfs2_wq);
> 
> The flush_workqueue() calls ocfs2_get_system_file_inode() to grab main_bm_inode.
> However, after ocfs2_release_system_inodes(), all the cached system inodes are gone,
> And the _ocfs2_get_system_file_inode() uses osb->sys_root_inode, which is already
> freed by ocfs2_release_system_inodes(). So we should move the
> flush_workqueue() calls to before ocfs2_release_system_inodes().
> 

Thanks, sashiko also has ponited out this issue.
I've fixed in v2 and now it is under testing. Will send out later.

Thanks,
Joseph