Re: [PATCH 01/10] iomap: factor out a iomap_last_written_block helper

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Sep 23, 2024 at 05:28:15PM +0200, Christoph Hellwig wrote:
> Split out a pice of logic from iomap_file_buffered_write_punch_delalloc
> that is useful for all iomap_end implementations.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/buffered-io.c | 13 ++-----------
>  include/linux/iomap.h  | 14 ++++++++++++++
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 11ea747228aeec..884891ac7a226c 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1280,7 +1280,6 @@ void iomap_file_buffered_write_punch_delalloc(struct inode *inode,
>  {
>  	loff_t			start_byte;
>  	loff_t			end_byte;
> -	unsigned int		blocksize = i_blocksize(inode);
>  
>  	if (iomap->type != IOMAP_DELALLOC)
>  		return;
> @@ -1289,16 +1288,8 @@ void iomap_file_buffered_write_punch_delalloc(struct inode *inode,
>  	if (!(iomap->flags & IOMAP_F_NEW))
>  		return;
>  
> -	/*
> -	 * start_byte refers to the first unused block after a short write. If
> -	 * nothing was written, round offset down to point at the first block in
> -	 * the range.
> -	 */
> -	if (unlikely(!written))
> -		start_byte = round_down(pos, blocksize);
> -	else
> -		start_byte = round_up(pos + written, blocksize);
> -	end_byte = round_up(pos + length, blocksize);
> +	start_byte = iomap_last_written_block(inode, pos, written);
> +	end_byte = round_up(pos + length, i_blocksize(inode));
>  
>  	/* Nothing to do if we've written the entire delalloc extent */
>  	if (start_byte >= end_byte)
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 4ad12a3c8bae22..e62df5d93f04de 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -256,6 +256,20 @@ static inline const struct iomap *iomap_iter_srcmap(const struct iomap_iter *i)
>  	return &i->iomap;
>  }
>  
> +/*
> + * Return the file offset for the first unused block after a short write.

the first *unchanged* block after a short write?

> + *
> + * If nothing was written, round offset down to point at the first block in

Might as well make explicit which variable we're operating on:
"...round @pos down..."

--D

> + * the range, else round up to include the partially written block.
> + */
> +static inline loff_t iomap_last_written_block(struct inode *inode, loff_t pos,
> +		ssize_t written)
> +{
> +	if (unlikely(!written))
> +		return round_down(pos, i_blocksize(inode));
> +	return round_up(pos + written, i_blocksize(inode));
> +}
> +
>  ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
>  		const struct iomap_ops *ops, void *private);
>  int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops);
> -- 
> 2.45.2
>