Re: [PATCH] ext4:Fix WARNING in ext4_journalled_invalidate_folio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ABBA deadlock between truncating thread and journal commit thread due to holding the folio lock while waiting for a transaction commit.
--

commit 0b3a826d64ad8fde652eef131d6bd13542e8206b
Author: shuo chen <1289151713@qq.com>

ext4:Fix WARNING in ext4_journalled_invalidate_folio

This commit modifies ext4_journalled_invalidate_folio() to handle -EBUSY
from __ext4_journalled_invalidate_folio() by waiting for the current
transaction to commit before retrying.

Closes: https://lore.kernel.org/linux-ext4/CA+0ovCgxFN3TLpdS_O_Z48sARuQ-WR-JLDCqQWEmd9ELtkzddA@mail.gmail.com/T/

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 26f0f9714f03c..e5e78d5551474 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3419,7 +3419,27 @@ static void ext4_journalled_invalidate_folio(struct folio *folio,
>  					   size_t offset,
>  					   size_t length)
>  {
> -	WARN_ON(__ext4_journalled_invalidate_folio(folio, offset, length) < 0);
> +	tid_t commit_tid;
> +	int ret;
> +	bool has_transaction;
> +	journal_t *journal = EXT4_JOURNAL(folio->mapping->host);
> +
> +	while (1) {
> +		ret = __ext4_journalled_invalidate_folio(folio, offset, length);
> +		if (ret != -EBUSY) {
> +			WARN_ON(ret < 0);
> +			return;
> +		}
> +		has_transaction = false;
> +		read_lock(&journal->j_state_lock);
> +		if (journal->j_committing_transaction) {
> +			commit_tid = journal->j_committing_transaction->t_tid;
> +			has_transaction = true;
> +		}
> +		read_unlock(&journal->j_state_lock);
> +		if (has_transaction)
> +			jbd2_log_wait_commit(journal, commit_tid);

[Severity: High]
Does waiting for a journal commit while holding the folio lock cause an ABBA
deadlock with the JBD2 commit thread?

The truncation thread enters this path with the folio locked. If it sleeps
here, it blocks while holding the folio lock.

Meanwhile, the main JBD2 commit thread loops over dirty folios during commit:

fs/ext4/super.c:ext4_journalled_submit_inode_data_buffers() {
    ...
	while ((folio = writeback_iter(mapping, &wbc, folio, &error))) {
		if (ext4_journalled_writepage_needs_redirty(jinode, folio))
			folio_redirty_for_writepage(&wbc, folio);
		folio_unlock(folio);
	}
    ...
}

The writeback_iter() function internally calls folio_lock(). If it reaches
the folio held by the truncation thread, the commit thread will block
waiting for the folio lock, while the truncation thread is blocked waiting
for the commit thread to finish, leading to a complete filesystem stall.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_D74F4B8A5FDC1BCFC23B8750428019635609@qq.com?part=1