Re: [PATCH] jbd2: Increase IO priority of checkpoint.

[Julian Sun <sunjunchao@bytedance.com>]

在 2025/8/27 04:55, Ritesh Harjani (IBM) 写道:
> Julian Sun <sunjunchao@bytedance.com> writes:
> 
>> In commit 6a3afb6ac6df ("jbd2: increase the journal IO's priority"),
>> the priority of IOs initiated by jbd2 has been raised, exempting them
>> from WBT throttling.
>> Checkpoint is also a crucial operation of jbd2. While no serious issues
>> have been observed so far, it should still be reasonable to exempt
>> checkpoint from WBT throttling.
>>
> 
> Interesting.. I was wondering whether we were able to observe any
> throttling for jbd2 log writes or for jbd2 checkpoint?
> Maybe It would have been nice, if we had some kind of data for this.

Good idea. But AFAICS wbt lacks of such a obversation mechanism now..>
> BTW - does it make sense for fastcommit path too maybe for non-tail
> fc write requests? I think it uses ext4_fc_submit_bh().

Yeah, I think so.
After a rough check of the code, the following code paths may result in 
high latency or even task hangs:
   1. fastcommit io is throttled by wbt or other block layer qos policies.
   2. jbd2_fc_wait_bufs() might wait for a long time while 
JBD2_FAST_COMMIT_ONGOING is set in journal->flags, and then 
jbd2_journal_commit_transaction() waits for the 
JBD2_FAST_COMMIT_ONGOING bit for a long time while holding the write 
lock of j_state_lock.
   3. start_this_handle() waits for read lock of j_state_lock which 
results in high latency or task hang.

Hi, Jan, please correct me if I'm missing anything.
> 
> -ritesh
> 
> 
>> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
>> ---
>>   fs/jbd2/checkpoint.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
>> index 38861ca04899..2d0719bf6d87 100644
>> --- a/fs/jbd2/checkpoint.c
>> +++ b/fs/jbd2/checkpoint.c
>> @@ -131,7 +131,7 @@ __flush_batch(journal_t *journal, int *batch_count)
>>   
>>   	blk_start_plug(&plug);
>>   	for (i = 0; i < *batch_count; i++)
>> -		write_dirty_buffer(journal->j_chkpt_bhs[i], REQ_SYNC);
>> +		write_dirty_buffer(journal->j_chkpt_bhs[i], JBD2_JOURNAL_REQ_FLAGS);
>>   	blk_finish_plug(&plug);
>>   
>>   	for (i = 0; i < *batch_count; i++) {
>> -- 
>> 2.20.1

Thanks,
-- 
Julian Sun <sunjunchao@bytedance.com>