Am 10.05.22 um 14:10 schrieb Greg KH:
On Tue, May 10, 2022 at 01:35:41PM +0200, Christian König wrote:
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.
I'll note that the original patch series for this described why this was
moved from debugfs to 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.
Why? Device names change all the time and should never be static. A
buffer name should just be a unique identifier in that directory, that's
all. No rules on the formatting of it unless for some reason the name
being the inode number was somehow being used in userspace for that
number?
My impression was that we documented that should have been a number, but
I might be wrong on this. And if it's not documented to be a number, I
think it should be.
The background is that you probably need to associate the DMA-buf with
some userspace structure for accounting and that becomes easier when you
can just put them into a radix.
Regards,
Christian.
thanks,
greg k-h
_______________________________________________
Linaro-mm-sig mailing list -- linaro-mm-sig@xxxxxxxxxxxxxxxx
To unsubscribe send an email to linaro-mm-sig-leave@xxxxxxxxxxxxxxxx
