On viernes, 29 de mayo de 2020 19:10:29 (CEST) Eric Anholt wrote:
On Fri, May 29, 2020 at 6:44 AM Rohan Garg rohan.garg@collabora.com wrote:
Hey Eric!
On jueves, 28 de mayo de 2020 20:45:24 (CEST) Eric Anholt wrote:
On Thu, May 28, 2020 at 10:06 AM Rohan Garg rohan.garg@collabora.com
wrote:
DRM_IOCTL_HANDLE_SET_LABEL lets you label buffers associated with a handle, making it easier to debug issues in userspace applications.
DRM_IOCTL_HANDLE_GET_LABEL lets you read the label associated with a buffer.
Changes in v2:
- Hoist the IOCTL up into the drm_driver framework
Changes in v3:
Introduce a drm_gem_set_label for drivers to use internally
in order to label a GEM object
Hoist string copying up into the IOCTL
Fix documentation
Move actual gem labeling into drm_gem_adopt_label
Changes in v4:
Refactor IOCTL call to only perform string duplication and move
all gem lookup logic into GEM specific call
Changes in v5:
Fix issues pointed out by kbuild test robot
Cleanup minor issues around kfree and out/err labels
Fixed API documentation issues
Rename to DRM_IOCTL_HANDLE_SET_LABEL
Introduce a DRM_IOCTL_HANDLE_GET_LABEL to read labels
Added some documentation for consumers of this IOCTL
Ensure that label's cannot be longer than PAGE_SIZE
Set a default label value
Drop useless warning
Properly return length of label to userspace even if
userspace did not allocate memory for label.
Changes in v6:
Wrap code to make better use of 80 char limit
Drop redundant copies of the label
Protect concurrent access to labels
Improved documentation
Refactor setter/getter ioctl's to be static
Return EINVAL when label length > PAGE_SIZE
Simplify code by calling the default GEM label'ing
function for all drivers using GEM
Do not error out when fetching empty labels
Refactor flags to the u32 type and add documentation
Refactor ioctls to use correct DRM_IOCTL{R,W,WR} macros
Return length of copied label to userspace
Signed-off-by: Rohan Garg rohan.garg@collabora.com
I don't think we should land this until it actually does something with the label, that feels out of the spirit of our uapi merge rules. I would suggest looking at dma_buf_set_name(), which would produce useful output in debugfs's /dma_buf/buf_info. But also presumably you have something in panfrost using this?
My current short term plan is to hook up glLabel to the labeling functionality in order for userspace applications to debug exactly which buffer objects could be causing faults in the kernel for speedier debugging.
So, something like "when a fault happens, dump/capture names of nearby objects"? That would certainly be nice to have!
Pretty much, yep. This seems to be a pretty common use case at the moment.
The more long term plan is to label each buffer with a unique id that we can correlate to the GL calls being flushed in order to be able to profile (a set of) GL calls on various platforms in order to aid driver developers with performance work. This could be something that we corelate on the userspace side with the help of perfetto by using this [1] patch that emits ftrace events.
I'm curious how BO names are part of profiling? Do you have the ability to sample instruction IP and use that to figure out where time is spent in shaders, or something?
The BO name itself isn't part of the profiling, but if we label our batch buffers, and then sample the performance counters with something like gfx-pps [1] and co relate them then we could figure out the cost of a batch of GL calls that were flushed to the hardware. The plan is to use DRM scheduler trace markers like this one [2] to figure out when jobs get run.
Cheers Rohan Garg
[1] https://gitlab.freedesktop.org/Fahien/gfx-pps/ [2] https://cgit.freedesktop.org/drm/drm-misc/commit/? id=c2c91828fbdbc5a31616f956834c85ab011392e1