Re: [PATCH v3 4/4] jbd2: store jinode dirty range in PAGE_SIZE units

[Li Chen <me@linux.beauty>]

Hi Jan,

 ---- On Tue, 03 Mar 2026 17:01:28 +0800  Jan Kara <jack@suse.cz> wrote --- 
 > On Mon 02-03-26 19:27:13, Jan Kara wrote:
 > > On Tue 24-02-26 17:24:33, Li Chen wrote:
 > > > jbd2_inode fields are updated under journal->j_list_lock, but some paths
 > > > read them without holding the lock (e.g. fast commit helpers and ordered
 > > > truncate helpers).
 > > > 
 > > > READ_ONCE() alone is not sufficient for the dirty range fields when they
 > > > are stored as loff_t because 32-bit platforms can observe torn loads.
 > > > Store the dirty range in PAGE_SIZE units as pgoff_t instead.
 > > > 
 > > > Use READ_ONCE() on the read side and WRITE_ONCE() on the write side for the
 > > > dirty range and i_flags to match the existing lockless access pattern.
 > > > 
 > > > Suggested-by: Jan Kara <jack@suse.cz>
 > > > Reviewed-by: Jan Kara <jack@suse.cz>
 > > > Signed-off-by: Li Chen <me@linux.beauty>
 > > ...
 > > > @@ -2654,15 +2655,20 @@ static int jbd2_journal_file_inode(handle_t *handle, struct jbd2_inode *jinode,
 > > >      jbd2_debug(4, "Adding inode %lu, tid:%d\n", jinode->i_vfs_inode->i_ino,
 > > >              transaction->t_tid);
 > > >  
 > > > +    start_page = (pgoff_t)(start_byte >> PAGE_SHIFT);
 > > > +    end_page = (pgoff_t)(end_byte >> PAGE_SHIFT);
 > > 
 > > MAX_LFS_SIZE on 32-bit is ULONG_MAX << PAGE_SHIFT and that's maximum file
 > > size. So we could do here end_page = DIV_ROUND_UP(end_byte, PAGE_SIZE) and
 > > just use end_page as exclusive end of a range to flush and get rid of
 > > special JBD2_INODE_DIRTY_RANGE_NONE value.
 > > 
 > > The problem with the scheme you use is that files of MAX_LFS_SIZE would be
 > > treated as having empty flush range...
 > 
 > Or perhaps even easier might be to set start_page to ULONG_MAX and end_page
 > to 0 and use start_page > end_page as an indication of empty range.

Great idea! Thanks for the detailed review.

I will update the range handling to avoid the sentinel collision.

Regards,
Li