Re: [RFC v7 3/7] ext4: fast commit: avoid waiting for FC_COMMITTING

Hi sashiko,

 ---- On Tue, 12 May 2026 10:25:25 +0800   <[email protected]> wrote --- 
 > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
 > - [High] Wait loops using `bit_waitqueue` test the wrong bit on 64-bit platforms, causing permanent deadlocks due to ignored wakeups for files with certain on-disk flags (e.g., encrypted or uncompressed files).
 > --
 > 
 > Note: The standard inline-diff format for this LKML report has been altered
 > to a free-form summary due to automated system recitation restrictions.
 > 
 > commit 6a0b3a4122c0edab1e98d39dcec0cf8e11e66265
 > Author: Li Chen <[email protected]>
 > ext4: fast commit: avoid waiting for FC_COMMITTING
 > 
 > This commit removes the block on FC_COMMITTING during fast commit inode
 > tracking to avoid deadlocks. It instead requeues modified inodes via a new
 > state flag and adds wait loops in the eviction path to block while a commit
 > is actively using the inode.
 > 
 > Looking at the newly added wait loop in ext4_fc_del() and the wakeup in
 > ext4_fc_cleanup(), could the use of bit_waitqueue() and DEFINE_WAIT_BIT()
 > with EXT4_STATE_FC_COMMITTING and EXT4_STATE_FC_FLUSHING_DATA cause tasks
 > to sleep forever on 64-bit systems?
 > 
 > On 64-bit platforms, the EXT4_INODE_BIT_FNS macro maps dynamic state flags
 > to bits 32 and above in the i_flags field. For example, the enum values for
 > EXT4_STATE_FC_COMMITTING (10) and EXT4_STATE_FC_FLUSHING_DATA (11) are
 > actually evaluated as bits 42 and 43 when accessed through the proper
 > ext4_test_inode_state() helpers.
 > 
 > However, the 64-bit fallback paths for the wait queues and wake_up_bit()
 > calls in this patch act directly on i_flags using the unshifted enum
 > values 10 and 11. In the context of the raw i_flags field, bit 10
 > corresponds to EXT4_NOCOMPR_FL, and bit 11 corresponds to EXT4_ENCRYPT_FL.
 > 
 > If a file has the encrypted or no-compress on-disk flags set, test_bit() in
 > the kernel's wake_bit_function() will return true because the on-disk flags
 > are set, rather than checking the intended dynamic state flags. Will this
 > cause wake_bit_function() to assume the condition is still pending and
 > ignore the wakeup, leaving the task in ext4_fc_del() deadlocked in an
 > uninterruptible sleep?
 > 
 > -- 
 > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
 > 

Thanks, good catch. This bug was already in 857d32f26181 ("ext4: rework
fast commit commit path"); this patch just moved the same wait/wake code
into ext4_fc_del().

I think it's cleaner to send a small standalone fix patch first, instead
of folding this into v8. I'll add a helper to map EXT4_STATE_* to the
real wait word/bit, and use it for both bit_waitqueue() and wake_up_bit()
so it matches ext4_test_inode_state() on 64-bit.

I'll look at the other two review mails tomorrow.

Li​