Re: [PATCH v4 3/8] dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE

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

On 11/4/26 12:09, Albert Esteve wrote:
> dma-heap already supports charging a specific cgroup at allocation
> time via charge_fd in DMA_HEAP_IOCTL_ALLOC. However, dma-bufs can
> originate from any exporter (DRM, V4L2, etc.), and the allocator may
> not know the final owner at allocation time.
> 
> Add DMA_BUF_IOCTL_XFER_CHARGE to transfer a dma-buf's memcg charge
> to a different cgroup after allocation. The target is identified by
> charge_fd, which may be a cgroupfd or a pidfd.
> 
> The transfer charges the new cgroup before uncharging the old one,
> preserving the original charge on failure. Concurrent transfers are
> serialized by the dma-buf reservation lock, so lockdep covers the
> locking order of the charge path early. The transfer implementation
> is generic (mem_cgroup_transfer_charge) and not dma-buf specific, so
> it can be reused by other subsystems.
> 
> Not all dma-buf exporters back their buffers with reclaimable system
> memory. Some represent device registers, VRAM, or carved-out pools
> where size is not a meaningful memory cost. The xfer ioctl returns
> -EOPNOTSUPP for buffers whose memcg is not set. Exporters opt in by
> passing a memcg through dma_buf_export_info at export time. Currently
> only the system heap sets it, other exporters can opt in as needed.
> 
> Usage example:
>   int cg_fd = open("/sys/fs/cgroup/clients/app1", O_RDONLY | O_DIRECTORY);
> 
>   struct dma_buf_transfer_charge xfer = {
>       .charge_fd = cg_fd,
>   };
>   ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer);
>   /* charge now on app1's cgroup */
>   close(cg_fd);
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  drivers/dma-buf/dma-buf.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/dma-buf.h      |  3 ++-
>  include/linux/memcontrol.h   | 13 +++++++++++++
>  include/uapi/linux/dma-buf.h | 26 ++++++++++++++++++++++++++
>  mm/memcontrol.c              | 42 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 125 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 837666273550..3bd26fbb1109 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -540,6 +540,45 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf,
>  }
>  #endif
>  
> +/* Transfer a dma-buf's memcg charge; see struct dma_buf_transfer_charge */
> +static long dma_buf_xfer_charge(struct dma_buf *dmabuf,
> +				const void __user *user_data)
> +{
> +	struct dma_buf_transfer_charge arg;
> +	struct mem_cgroup *memcg;
> +	unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
> +	int ret = 0;
> +
> +	if (copy_from_user(&arg, user_data, sizeof(arg)))
> +		return -EFAULT;
> +
> +	if (!IS_ENABLED(CONFIG_MEMCG))
> +		return 0;
> +
> +	if (arg.flags != 0)
> +		return -EINVAL;
> +
> +	memcg = mem_cgroup_get_from_fd(arg.charge_fd);
> +	if (IS_ERR(memcg))
> +		return PTR_ERR(memcg);
> +
> +	dma_resv_lock(dmabuf->resv, NULL);
> +
> +	if (!dmabuf->memcg) {
> +		ret = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	if (dmabuf->memcg != memcg &&
> +	    !mem_cgroup_transfer_charge(&dmabuf->memcg, memcg, MEMCG_DMABUF, nr_pages))

Please move the calculation of nr_pages after taking the dma_resv_lock().

We currently don't have anybody dynamically changing DMA-buf sizes but just to be on the save side.

With that done Reviewed-by: Christian König <christian.koenig@amd.com>

Regards,
Christian.

> +		ret = -ENOMEM;
> +
> +out:
> +	dma_resv_unlock(dmabuf->resv);
> +	mem_cgroup_put(memcg);
> +	return ret;
> +}
> +
>  static long dma_buf_ioctl(struct file *file,
>  			  unsigned int cmd, unsigned long arg)
>  {
> @@ -590,6 +629,9 @@ static long dma_buf_ioctl(struct file *file,
>  		return dma_buf_import_sync_file(dmabuf, (const void __user *)arg);
>  #endif
>  
> +	case DMA_BUF_IOCTL_XFER_CHARGE:
> +		return dma_buf_xfer_charge(dmabuf, (const void __user *)arg);
> +
>  	default:
>  		return -ENOTTY;
>  	}
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 85dd747c9b36..b01149d26b44 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -507,7 +507,8 @@ struct dma_buf_attachment {
>   * @priv:	Attach private data of allocator to this buffer
>   * @memcg:	memcg to charge for this buffer's memory. Set only for
>   *		buffers backed by system memory where @size reflects the
> - *		actual allocation in bytes.
> + *		actual allocation in bytes. Enables charge transfer via
> + *		DMA_BUF_IOCTL_XFER_CHARGE.
>   *
>   * This structure holds the information required to export the buffer. Used
>   * with dma_buf_export() only.
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index f8ec751b7d7b..af02d9777fe9 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -676,6 +676,11 @@ struct mem_cgroup *mem_cgroup_get_from_fd(int fd);
>  bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp,
>  			       unsigned int nr_pages);
>  
> +bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
> +				struct mem_cgroup *new_memcg,
> +				enum memcg_stat_item item,
> +				unsigned int nr_pages);
> +
>  void __mem_cgroup_uncharge(struct folio *folio);
>  
>  /**
> @@ -1197,6 +1202,14 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg,
>  {
>  }
>  
> +static inline bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
> +					      struct mem_cgroup *new_memcg,
> +					      enum memcg_stat_item item,
> +					      unsigned int nr_pages)
> +{
> +	return true;
> +}
> +
>  static inline void mem_cgroup_uncharge(struct folio *folio)
>  {
>  }
> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
> index e827c9d20c5d..0aaf2e7d593b 100644
> --- a/include/uapi/linux/dma-buf.h
> +++ b/include/uapi/linux/dma-buf.h
> @@ -168,6 +168,31 @@ struct dma_buf_import_sync_file {
>  	__s32 fd;
>  };
>  
> +/**
> + * struct dma_buf_transfer_charge - Transfer a dma-buf's memcg charge
> + *
> + * Any holder of a dma-buf file descriptor can perform a
> + * DMA_BUF_IOCTL_XFER_CHARGE to attribute the buffer's memory
> + * cost to a specific cgroup, or move it from one cgroup to
> + * another.  The ioctl charges the new cgroup before uncharging
> + * the old one atomically.  On failure the original charge is
> + * preserved.  If the buffer has no existing memcg charge, the
> + * ioctl returns -EOPNOTSUPP.
> + */
> +struct dma_buf_transfer_charge {
> +	/** @flags: Reserved, must be zero */
> +	__u32 flags; /* unused for now */
> +	/**
> +	 * @charge_fd: File descriptor identifying the target cgroup.
> +	 *
> +	 * May be either a pidfd, in which case the charge moves to the
> +	 * memory cgroup of the referenced process, or a cgroupfd obtained
> +	 * by opening a cgroup v2 directory, in which case the charge moves
> +	 * directly to that cgroup's memory controller.
> +	 */
> +	__u32 charge_fd;
> +};
> +
>  #define DMA_BUF_BASE		'b'
>  #define DMA_BUF_IOCTL_SYNC	_IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
>  
> @@ -179,5 +204,6 @@ struct dma_buf_import_sync_file {
>  #define DMA_BUF_SET_NAME_B	_IOW(DMA_BUF_BASE, 1, __u64)
>  #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE	_IOWR(DMA_BUF_BASE, 2, struct dma_buf_export_sync_file)
>  #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE	_IOW(DMA_BUF_BASE, 3, struct dma_buf_import_sync_file)
> +#define DMA_BUF_IOCTL_XFER_CHARGE	_IOW(DMA_BUF_BASE, 4, struct dma_buf_transfer_charge)
>  
>  #endif
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 17fdefc56b97..3e717b5663c8 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5752,6 +5752,48 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages)
>  }
>  EXPORT_SYMBOL_GPL(mem_cgroup_cancel_dmabuf);
>  
> +/**
> + * mem_cgroup_transfer_charge - move a charge between memcgs
> + * @ptr_memcg: pointer to the memcg to transfer from
> + * @new_memcg: target memcg; caller must hold a reference
> + * @item: stat counter to adjust
> + * @nr_pages: number of pages to transfer
> + *
> + * Transfers @nr_pages of charge from *@ptr_memcg to @new_memcg and, on
> + * success, stores @new_memcg in *@ptr_memcg.
> + *
> + * Updates to *@ptr_memcg must be serialized by the caller.
> + * Returns %true on success, %false if the charge against @new_memcg
> + * failed.
> + */
> +bool mem_cgroup_transfer_charge(struct mem_cgroup **ptr_memcg,
> +				struct mem_cgroup *new_memcg,
> +				enum memcg_stat_item item,
> +				unsigned int nr_pages)
> +{
> +	struct mem_cgroup *old_memcg = *ptr_memcg;
> +
> +	if (!new_memcg || mem_cgroup_disabled())
> +		return true;
> +
> +	if (try_charge(new_memcg, GFP_KERNEL, nr_pages))
> +		return false;
> +
> +	mod_memcg_state(new_memcg, item, nr_pages);
> +	css_get(&new_memcg->css);
> +	*ptr_memcg = new_memcg;
> +
> +	if (old_memcg) {
> +		/* Uncharge old cgroup */
> +		mod_memcg_state(old_memcg, item, -nr_pages);
> +		if (!mem_cgroup_is_root(old_memcg))
> +			refill_stock(old_memcg, nr_pages);
> +		mem_cgroup_put(old_memcg);
> +	}
> +
> +	return true;
> +}
> +
>  /**
>   * mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
>   * @dmabuf: buffer to uncharge
>