Re: [PATCH] dmabuf: ensure unique directory name for dmabuf stats

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

Am 10.05.22 um 13:00 schrieb Greg KH:
> On Tue, May 10, 2022 at 03:53:32PM +0530, Charan Teja Kalla wrote:
>> The dmabuf file uses get_next_ino()(through dma_buf_getfile() ->
>> alloc_anon_inode()) to get an inode number and uses the same as a
>> directory name under /sys/kernel/dmabuf/buffers/<ino>. This directory is
>> used to collect the dmabuf stats and it is created through
>> dma_buf_stats_setup(). At current, failure to create this directory
>> entry can make the dma_buf_export() to fail.
>>
>> Now, as the get_next_ino() can definitely give a repetitive inode no
>> causing the directory entry creation to fail with -EEXIST. This is a
>> problem on the systems where dmabuf stats functionality is enabled on
>> the production builds can make the dma_buf_export(), though the dmabuf
>> memory is allocated successfully, to fail just because it couldn't
>> create stats entry.
> Then maybe we should not fail the creation path of the kobject fails to
> be created?  It's just for debugging, it should be fine if the creation
> of it isn't there.

Well if it's just for debugging then it should be under debugfs and not 
sysfs.

>> This issue we are able to see on the snapdragon system within 13 days
>> where there already exists a directory with inode no "122602" so
>> dma_buf_stats_setup() failed with -EEXIST as it is trying to create
>> the same directory entry.
>>
>> To make the directory entry as unique, append the inode creation time to
>> the inode. With this change the stats directory entries will be in the
>> format of: /sys/kernel/dmabuf/buffers/<inode no>-<inode creation time in
>> secs>.
> As you are changing the format here, shouldn't the Documentation/ABI/
> entry for this also be changed?

As far as I can see that is even an UAPI break, not sure if we can allow 
that.

> And what's to keep the seconds field from also being the same?

Well exporting two DMA-bufs with the same ino in the same nanosecond 
should be basically impossible, but I would rather opt for using a 64bit 
atomic in that function.

This should be 100% UAPI compatible and even if we manage to create one 
buffer ever ns we need ~500years to wrap around.

Regards,
Christian.

>
> thanks,
>
> greg k-h