Re: [PATCH v2 2/4] jbd2: Precompute number of transaction descriptor blocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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
> 





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux