Re: [PATCH v3 04/22] ext4: add iomap address space operations for buffered I/O

[Jan Kara]

On Wed 22-04-26 10:10:24, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> Introduce initial support for iomap in the buffered I/O path for regular
> files on ext4.
> 
>   - Add a new inode state flag EXT4_STATE_BUFFERED_IOMAP to indicate the
>     inode uses iomap instead of buffer_head for buffered I/O
>   - Add helper ext4_inode_buffered_iomap() to check the flag
>   - Add new address space operations ext4_iomap_aops with callbacks that
>     will use generic iomap implementations
>   - Add ext4_iomap_aops to ext4_set_aops() when the flag is set
> 
> The following callbacks(read_folio(), readahead(), writepages()) are
> provided as placeholders and will be implemented in later patches.
> 
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ext4/ext4.h  |  7 +++++++
>  fs/ext4/inode.c | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 9e4353432325..fe3491ad2129 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1972,6 +1972,7 @@ enum {
>  	EXT4_STATE_FC_COMMITTING,	/* Fast commit ongoing */
>  	EXT4_STATE_FC_FLUSHING_DATA,	/* Fast commit flushing data */
>  	EXT4_STATE_ORPHAN_FILE,		/* Inode orphaned in orphan file */
> +	EXT4_STATE_BUFFERED_IOMAP,	/* Inode use iomap for buffered IO */
>  };
>  
>  #define EXT4_INODE_BIT_FNS(name, field, offset)				\
> @@ -2040,6 +2041,12 @@ static inline bool ext4_inode_orphan_tracked(struct inode *inode)
>  		!list_empty(&EXT4_I(inode)->i_orphan);
>  }
>  
> +/* Whether the inode pass through the iomap infrastructure for buffered I/O */
> +static inline bool ext4_inode_buffered_iomap(struct inode *inode)
> +{
> +	return ext4_test_inode_state(inode, EXT4_STATE_BUFFERED_IOMAP);
> +}
> +
>  /*
>   * Codes for operating systems
>   */
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 59405a95ecfc..9e9f421888ed 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3908,6 +3908,22 @@ const struct iomap_ops ext4_iomap_report_ops = {
>  	.iomap_begin = ext4_iomap_begin_report,
>  };
>  
> +static int ext4_iomap_read_folio(struct file *file, struct folio *folio)
> +{
> +	return 0;
> +}
> +
> +static void ext4_iomap_readahead(struct readahead_control *rac)
> +{
> +
> +}
> +
> +static int ext4_iomap_writepages(struct address_space *mapping,
> +				 struct writeback_control *wbc)
> +{
> +	return 0;
> +}
> +
>  /*
>   * For data=journal mode, folio should be marked dirty only when it was
>   * writeably mapped. When that happens, it was already attached to the
> @@ -3994,6 +4010,20 @@ static const struct address_space_operations ext4_da_aops = {
>  	.swap_activate		= ext4_iomap_swap_activate,
>  };
>  
> +static const struct address_space_operations ext4_iomap_aops = {
> +	.read_folio		= ext4_iomap_read_folio,
> +	.readahead		= ext4_iomap_readahead,
> +	.writepages		= ext4_iomap_writepages,
> +	.dirty_folio		= iomap_dirty_folio,
> +	.bmap			= ext4_bmap,
> +	.invalidate_folio	= iomap_invalidate_folio,
> +	.release_folio		= iomap_release_folio,
> +	.migrate_folio		= filemap_migrate_folio,
> +	.is_partially_uptodate  = iomap_is_partially_uptodate,
> +	.error_remove_folio	= generic_error_remove_folio,
> +	.swap_activate		= ext4_iomap_swap_activate,
> +};
> +
>  static const struct address_space_operations ext4_dax_aops = {
>  	.writepages		= ext4_dax_writepages,
>  	.dirty_folio		= noop_dirty_folio,
> @@ -4015,6 +4045,8 @@ void ext4_set_aops(struct inode *inode)
>  	}
>  	if (IS_DAX(inode))
>  		inode->i_mapping->a_ops = &ext4_dax_aops;
> +	else if (ext4_inode_buffered_iomap(inode))
> +		inode->i_mapping->a_ops = &ext4_iomap_aops;
>  	else if (test_opt(inode->i_sb, DELALLOC))
>  		inode->i_mapping->a_ops = &ext4_da_aops;
>  	else
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR