On Wed, Jul 24 2024, Jan Kara wrote:
> On Tue 23-07-24 16:43:59, Luis Henriques (SUSE) wrote:
>> Function ext4_wait_for_tail_page_commit() assumes that '0' is not a valid
>> value for transaction IDs, which is incorrect. Don't assume that and invoke
>> jbd2_log_wait_commit() if the journal had a committing transaction instead.
>>
>> Signed-off-by: Luis Henriques (SUSE) <luis.henriques@xxxxxxxxx>
>> ---
>> fs/ext4/inode.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 941c1c0d5c6e..e65fc2086701 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -5279,8 +5279,9 @@ static void ext4_wait_for_tail_page_commit(struct inode *inode)
>> {
>> unsigned offset;
>> journal_t *journal = EXT4_SB(inode->i_sb)->s_journal;
>> - tid_t commit_tid = 0;
>> + tid_t commit_tid;
>> int ret;
>> + bool has_transaction = false;
>>
>> offset = inode->i_size & (PAGE_SIZE - 1);
>> /*
>> @@ -5305,12 +5306,13 @@ static void ext4_wait_for_tail_page_commit(struct inode *inode)
>> folio_put(folio);
>> if (ret != -EBUSY)
>> return;
>> - commit_tid = 0;
>
> We should set "has_transaction = false" here to make things work properly
> when looping... Otherwise looks good.
Ah! Good point. I'll fix that, thanks!
Cheers,
--
Luís
>
> Honza
>
>> read_lock(&journal->j_state_lock);
>> - if (journal->j_committing_transaction)
>> + if (journal->j_committing_transaction) {
>> commit_tid = journal->j_committing_transaction->t_tid;
>> + has_transaction = true;
>> + }
>> read_unlock(&journal->j_state_lock);
>> - if (commit_tid)
>> + if (has_transaction)
>> jbd2_log_wait_commit(journal, commit_tid);
>> }
>> }
>>
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
