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