Re: [PATCH v4 2/4] dmabuf: Implement copy_file_range callback for dmabuf direct I/O prep

["Christian König" <christian.koenig@amd.com>]

On 6/3/25 11:52, wangtao wrote:
> First determine if dmabuf reads from or writes to the file.
> Then call exporter's rw_file callback function.
> 
> Signed-off-by: wangtao <tao.wangtao@honor.com>
> ---
>  drivers/dma-buf/dma-buf.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/dma-buf.h   | 16 ++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 5baa83b85515..fc9bf54c921a 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -523,7 +523,38 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
>  	spin_unlock(&dmabuf->name_lock);
>  }
>  
> +static ssize_t dma_buf_rw_file(struct dma_buf *dmabuf, loff_t my_pos,
> +	struct file *file, loff_t pos, size_t count, bool is_write)
> +{
> +	if (!dmabuf->ops->rw_file)
> +		return -EINVAL;
> +
> +	if (my_pos >= dmabuf->size)
> +		count = 0;
> +	else
> +		count = min_t(size_t, count, dmabuf->size - my_pos);
> +	if (!count)
> +		return 0;
> +
> +	return dmabuf->ops->rw_file(dmabuf, my_pos, file, pos, count, is_write);
> +}
> +
> +static ssize_t dma_buf_copy_file_range(struct file *file_in, loff_t pos_in,
> +	struct file *file_out, loff_t pos_out,
> +	size_t count, unsigned int flags)
> +{
> +	if (is_dma_buf_file(file_in) && file_out->f_op->write_iter)
> +		return dma_buf_rw_file(file_in->private_data, pos_in,
> +				file_out, pos_out, count, true);
> +	else if (is_dma_buf_file(file_out) && file_in->f_op->read_iter)
> +		return dma_buf_rw_file(file_out->private_data, pos_out,
> +				file_in, pos_in, count, false);
> +	else
> +		return -EINVAL;
> +}
> +
>  static const struct file_operations dma_buf_fops = {
> +	.fop_flags = FOP_MEMORY_FILE,
>  	.release	= dma_buf_file_release,
>  	.mmap		= dma_buf_mmap_internal,
>  	.llseek		= dma_buf_llseek,
> @@ -531,6 +562,7 @@ static const struct file_operations dma_buf_fops = {
>  	.unlocked_ioctl	= dma_buf_ioctl,
>  	.compat_ioctl	= compat_ptr_ioctl,
>  	.show_fdinfo	= dma_buf_show_fdinfo,
> +	.copy_file_range = dma_buf_copy_file_range,
>  };
>  
>  /*
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 36216d28d8bd..d3636e985399 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -22,6 +22,7 @@
>  #include <linux/fs.h>
>  #include <linux/dma-fence.h>
>  #include <linux/wait.h>
> +#include <uapi/linux/dma-buf.h>
>  
>  struct device;
>  struct dma_buf;
> @@ -285,6 +286,21 @@ struct dma_buf_ops {
>  
>  	int (*vmap)(struct dma_buf *dmabuf, struct iosys_map *map);
>  	void (*vunmap)(struct dma_buf *dmabuf, struct iosys_map *map);
> +
> +	/**
> +	 * @rw_file:
> +	 *
> +	 * If an Exporter needs to support Direct I/O file operations, it can
> +	 * implement this optional callback. The exporter must verify that no
> +	 * other objects hold the sg_table, ensure exclusive access to the
> +	 * dmabuf's sg_table, and only then proceed with the I/O operation.

Explain why and not what. E.g. something like "Allows direct I/O between this DMA-buf and the file".

Completely drop mentioning the sg_table, that is irrelevant. Exclusive access depends on how the exporter implements the whole thing.

Regards,
Christian.

> +	 *
> +	 * Returns:
> +	 *
> +	 * 0 on success or a negative error code on failure.
> +	 */
> +	ssize_t (*rw_file)(struct dma_buf *dmabuf, loff_t my_pos,
> +		struct file *file, loff_t pos, size_t count, bool is_write);
>  };
>  
>  /**