on 6/25/2024 7:07 PM, Jan Kara wrote:
> On Tue 25-06-24 17:31:15, Kemeng Shi wrote:
>> on 6/25/2024 1:01 AM, Jan Kara wrote:
>>> Instead of computing the number of descriptor blocks a transaction can
>>> have each time we need it (which is currently when starting each
>>> transaction but will become more frequent later) precompute the number
>>> once during journal initialization together with maximum transaction
>>> size. We perform the precomputation whenever journal feature set is
>>> updated similarly as for computation of
>>> journal->j_revoke_records_per_block.
>>>
>>> CC: stable@xxxxxxxxxxxxxxx
>>> Signed-off-by: Jan Kara <jack@xxxxxxx>
>>> ---
>>> fs/jbd2/journal.c | 61 ++++++++++++++++++++++++++++++++-----------
>>> fs/jbd2/transaction.c | 24 +----------------
>>> include/linux/jbd2.h | 7 +++++
>>> 3 files changed, 54 insertions(+), 38 deletions(-)
>>>
>>> +static int jbd2_descriptor_blocks_per_trans(journal_t *journal)
>>> +{
>>> + int tag_space = journal->j_blocksize - sizeof(journal_header_t);
>>> + int tags_per_block;
>>> +
>>> + /* Subtract UUID */
>>> + tag_space -= 16;
>>> + if (jbd2_journal_has_csum_v2or3(journal))
>>> + tag_space -= sizeof(struct jbd2_journal_block_tail);
>>> + /* Commit code leaves a slack space of 16 bytes at the end of block */
>>> + tags_per_block = (tag_space - 16) / journal_tag_bytes(journal);
>>> + /*
>>> + * Revoke descriptors are accounted separately so we need to reserve
>>> + * space for commit block and normal transaction descriptor blocks.
>>> + */
>>> + return 1 + DIV_ROUND_UP(jbd2_journal_get_max_txn_bufs(journal),
>>> + tags_per_block);
>>> +}
>> The change looks good to me. I wonder if the original calculation of
>> number of JBD2_DESCRIPTOR_BLOCK blocks is correct.
>> In my opinion, it should be:
>> DIV_ROUND_UP(jbd2_journal_get_max_txn_bufs(journal), tags_per_block *+ 1*)
>> Assume max_txn_bufs is 6, tags_per_block is 1, then we have one tag block
>> after each JBD2_DESCRIPTOR_BLOCK block. Then we could get 3
>> JBD2_DESCRIPTOR_BLOCK block at most rather than 6.
>> Please let me konw if I miss something, this confused me for sometime...
>
> So you are correct that the expression is overestimating the number of
> descriptor blocks required, essentially because we don't need descriptor
> blocks for descriptor blocks. But given tags_per_block is at least over 60,
> in common configurations over 250, this imprecision is not really
> substantial and I prefer a simple to argue about upper estimate...
Right, I agree with this. Thanks for explanation.
>
> Honza
>
