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

[Jan Kara]

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@xxxxxxx>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
> 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...

								Honza

-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR