Re: [PATCH v7 3/9] dma-buf: Provide dma_buf_set_name()

[Matt Evans <matt@ozlabs.org>]

On 25/09/2026 09:39, Christian König wrote:
> 
> 
> On 9/24/26 17:21, Matt Evans wrote:
>> Allow exporters to set the name of buffers, during export or for
>> subsequent updates.  This is useful to express buffer-specific debug
>> information, indicate state particular to the exporter's usage, etc.
>> Only exporters and userspace are expected to set names; it isn't
>> sensible for importers to do it.
>>
>> Originally, the static dma_buf_set_name() was used for
>> DMA_BUF_SET_NAME ioctls, taking a __user string parameter.  Export
>> this function as a generic set-name helper, taking a kernel-allocated
>> string.  The SET_NAME ioctls now locally duplicate the __user string,
>> and use the new function to set the name.
>>
>> Signed-off-by: Matt Evans <matt@ozlabs.org>
>> ---
>>   drivers/dma-buf/dma-buf.c | 78 ++++++++++++++++++++++++---------------
>>   include/linux/dma-buf.h   |  2 +
>>   2 files changed, 50 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 4c9add51f9ef..e4ec504e9288 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -404,34 +404,6 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>>   	return events;
>>   }
>>   
>> -/**
>> - * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
>> - * It could support changing the name of the dma-buf if the same
>> - * piece of memory is used for multiple purpose between different devices.
>> - *
>> - * @dmabuf: [in]     dmabuf buffer that will be renamed.
>> - * @buf:    [in]     A piece of userspace memory that contains the name of
>> - *                   the dma-buf.
>> - *
>> - * Returns 0 on success. If the dma-buf buffer is already attached to
>> - * devices, return -EBUSY.
>> - *
>> - */
>> -static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
>> -{
>> -	char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
>> -
>> -	if (IS_ERR(name))
>> -		return PTR_ERR(name);
>> -
>> -	spin_lock(&dmabuf->name_lock);
>> -	kfree(dmabuf->name);
>> -	dmabuf->name = name;
>> -	spin_unlock(&dmabuf->name_lock);
>> -
>> -	return 0;
>> -}
>> -
>>   #if IS_ENABLED(CONFIG_SYNC_FILE)
>>   static long dma_buf_export_sync_file(struct dma_buf *dmabuf,
>>   				     void __user *user_data)
>> @@ -577,8 +549,19 @@ static long dma_buf_ioctl(struct file *file,
>>   		return ret;
>>   
>>   	case DMA_BUF_SET_NAME_A:
>> -	case DMA_BUF_SET_NAME_B:
>> -		return dma_buf_set_name(dmabuf, (const char __user *)arg);
>> +	case DMA_BUF_SET_NAME_B: {
>> +		char *name = strndup_user((const char __user *)arg,
>> +					  DMA_BUF_NAME_LEN);
>> +
>> +		if (IS_ERR(name))
>> +			return PTR_ERR(name);
>> +
>> +		ret = dma_buf_set_name(dmabuf, name);
>> +		if (ret)
>> +			kfree(name);
>> +
>> +		return ret;
>> +	}
>>   
>>   #if IS_ENABLED(CONFIG_SYNC_FILE)
>>   	case DMA_BUF_IOCTL_EXPORT_SYNC_FILE:
>> @@ -874,6 +857,41 @@ void dma_buf_put(struct dma_buf *dmabuf)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
>>   
>> +/**
>> + * dma_buf_set_name - Set a dmabuf's name
>> + * Intended to be used by the exporter to set a name for debug
>> + * purposes.  This can also change an existing name if the same piece
>> + * of memory is used for multiple purposes over time.
> 
> Add something like "Importers should *NOT* use this function to overwrite the name." here and add some note under "DOC: locking convention" how this function is supposed to be called.
> 
> Apart from that looks good to me,

Thanks, Christian.  Good point, done.

Re the locking convention, it appears dma_buf_set_name() is agnostic to 
whether resv is held or not.  However, rather than adding that as a new 
category to the 'DOC: locking convention', I've put this function in the 
existing "don't hold resv" section.

Whilst conservative, this matches existing (and this series's) usage, 
and can then be relaxed in future.  If you think a new "resv is 
permitted but not required to be held" category of API doc is better, do 
say.

Thanks,


Matt


> Christian.
> 
>> + *
>> + * @dmabuf: [in]     dmabuf buffer that will be renamed.
>> + * @name:   [in]     The name of the dma-buf, allocated with kmalloc() or
>> + *                   similar.  This takes ownership of the allocation
>> + *                   on success, which will be kfree()d when the
>> + *                   dmabuf is released or a new name assigned.
>> + *
>> + * Returns 0 on success, -EINVAL if the name is NULL, or -E2BIG if the
>> + * name (including terminator) exceeds DMA_BUF_NAME_LEN.
>> + */
>> +int dma_buf_set_name(struct dma_buf *dmabuf, char *name)
>> +{
>> +	if (!name)
>> +		return -EINVAL;
>> +
>> +	/* dmabuffs_dname() won't use the string if the length
>> +	 * (including terminator) exceeds DMA_BUF_NAME_LEN:
>> +	 */
>> +	if (strlen(name) >= DMA_BUF_NAME_LEN)
>> +		return -E2BIG;
>> +
>> +	spin_lock(&dmabuf->name_lock);
>> +	kfree(dmabuf->name);
>> +	dmabuf->name = name;
>> +	spin_unlock(&dmabuf->name_lock);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(dma_buf_set_name, "DMA_BUF");
>> +
>>   static int dma_buf_wrap_sg_table(struct sg_table **sg_table)
>>   {
>>   	struct scatterlist *to_sg, *from_sg;
>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>> index d15b2b31d3c9..952a2c196ad4 100644
>> --- a/include/linux/dma-buf.h
>> +++ b/include/linux/dma-buf.h
>> @@ -571,6 +571,8 @@ void dma_buf_fd_install(struct dma_buf *dmabuf, int fd);
>>   struct dma_buf *dma_buf_get(int fd);
>>   void dma_buf_put(struct dma_buf *dmabuf);
>>   
>> +int dma_buf_set_name(struct dma_buf *dmabuf, char *name);
>> +
>>   struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
>>   					enum dma_data_direction);
>>   void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
>