Re: [PATCH v2 4/8] ext4/jbd2: convert jbd2_journal_blocks_per_page() to support large folio

Jan Kara
2025-05-21T10:31:26Z

On Tue 20-05-25 20:46:51, Zhang Yi wrote:
> On 2025/5/20 4:16, Jan Kara wrote:
> > On Mon 12-05-25 14:33:15, Zhang Yi wrote:
> >> From: Zhang Yi <yi.zhang@huawei.com>
> >>
> >> jbd2_journal_blocks_per_page() returns the number of blocks in a single
> >> page. Rename it to jbd2_journal_blocks_per_folio() and make it returns
> >> the number of blocks in the largest folio, preparing for the calculation
> >> of journal credits blocks when allocating blocks within a large folio in
> >> the writeback path.
> >>
> >> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> > ...
> >> @@ -2657,9 +2657,10 @@ void jbd2_journal_ack_err(journal_t *journal)
> >> write_unlock(&journal->j_state_lock);
> >> }
> >>
> >> -int jbd2_journal_blocks_per_page(struct inode *inode)
> >> +int jbd2_journal_blocks_per_folio(struct inode *inode)
> >> {
> >> - return 1 << (PAGE_SHIFT - inode->i_sb->s_blocksize_bits);
> >> + return 1 << (PAGE_SHIFT + mapping_max_folio_order(inode->i_mapping) -
> >> + inode->i_sb->s_blocksize_bits);
> >> }
> >
> > FWIW this will result in us reserving some 10k transaction credits for 1k
> > blocksize with maximum 2M folio size. That is going to create serious
> > pressure on the journalling machinery. For now I guess we are fine
>
> Oooh, indeed, you are right, thanks a lot for pointing this out. As you
> mentioned in patch 5, the credits calculation I proposed was incorrect,
> I thought it wouldn't require too many credits.
>
> I believe it is risky to mount a filesystem with a small journal space
> and a large number of block groups. For example, if we build an image
> with a 1K block size and a 1MB journal on a 20GB disk (which contains
> 2,540 groups), it will require 2,263 credits, exceeding the available
> journal space.
>
> For now, I'm going to disable large folio support on the filesystem with
> limited journal space. i.e., when the return value of
> ext4_writepage_trans_blocks() is greater than
> jbd2_max_user_trans_buffers(journal) / 2, ext4_should_enable_large_folio()
> return false, thoughts?

Yep, looks like a good stopgap solution for now.

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR