On 10/18/23 21:17, Gurchetan Singh wrote: > There are two problems with the current method of determining the > virtio-gpu debug name. > > 1) TASK_COMM_LEN is defined to be 16 bytes only, and this is a > Linux kernel idiom (see PR_SET_NAME + PR_GET_NAME). Though, > Android/FreeBSD get around this via setprogname(..)/getprogname(..) > in libc. > > On Android, names longer than 16 bytes are common. For example, > one often encounters a program like "com.android.systemui". > > The virtio-gpu spec allows the debug name to be up to 64 bytes, so > ideally userspace should be able to set debug names up to 64 bytes. > > 2) The current implementation determines the debug name using whatever > task initiated virtgpu. This is could be a "RenderThread" of a > larger program, when we actually want to propagate the debug name > of the program. > > To fix these issues, add a new CONTEXT_INIT param that allows userspace > to set the debug name when creating a context. > > It takes a null-terminated C-string as the param value. The length of the > string (excluding the terminator) **should** be <= 64 bytes. Otherwise, > the debug_name will be truncated to 64 bytes. > > Link to open-source userspace: > https://android-review.googlesource.com/c/platform/hardware/google/gfxstream/+/2787176 > > Signed-off-by: Gurchetan Singh <gurchetansingh@xxxxxxxxxxxx> > Reviewed-by: Josh Simonot <josh.simonot@xxxxxxxxx> > --- > Fixes suggested by Dmitry Osipenko > v2: > - Squash implementation and UAPI change into one commit > - Avoid unnecessary casts > - Use bool when necessary > v3: > - Use DEBUG_NAME_MAX_LEN - 1 when copying string > > drivers/gpu/drm/virtio/virtgpu_drv.h | 5 ++++ > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 39 ++++++++++++++++++++++---- > include/uapi/drm/virtgpu_drm.h | 2 ++ > 3 files changed, 40 insertions(+), 6 deletions(-) ... > + ret = strncpy_from_user(vfpriv->debug_name, > + u64_to_user_ptr(value), > + DEBUG_NAME_MAX_LEN - 1); > + > + if (ret < 0) { > + ret = -EFAULT; > + goto out_unlock; > + } The strncpy_from_user() itself returns -EFAULT. I changed code to return the "ret" directly and applied to misc-next. Gerd, let us know if have any objections to this patch. -- Best regards, Dmitry
