From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E96D0C4332F for ; Tue, 31 Oct 2023 15:55:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1B1E310E544; Tue, 31 Oct 2023 15:55:25 +0000 (UTC) Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) by gabe.freedesktop.org (Postfix) with ESMTPS id B9A5010E53D for ; Tue, 31 Oct 2023 15:55:21 +0000 (UTC) Received: by mail-ej1-x62f.google.com with SMTP id a640c23a62f3a-9c603e235d1so927895166b.3 for ; Tue, 31 Oct 2023 08:55:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1698767720; x=1699372520; darn=lists.freedesktop.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=2A4Wc6DGVI3weUYWv14MisK09Ufaz4uvtIUn25HRO6s=; b=Ji7NNR4L4n+QTDgEe9ZBQpTfxCdjtPXgEBumTBsqPo++zP49idM/zWoXOuolEQ1o/Q GJrORwXFqskN9higfKOIZCLkGjSExX0WsMb//RelKNA4ihUDBDJHLaF8oCh8u46FbzNL hMboqHm+Zv6lD572HnZ9VzpsG1ZRezw45s0fw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698767720; x=1699372520; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=2A4Wc6DGVI3weUYWv14MisK09Ufaz4uvtIUn25HRO6s=; b=lMnOMtVAs2YbW75/mPc/1wHYjTrpeWQbvlxiI4cWZ8cEAgzOQ1/DDZdSovNNTXcZH+ 5ypM7y3JXMkoWR1zJAw6zX6WzgHTh4R9zmAm1zTM8MV4NdH2wSQNVhAdi+dOwaTNcOF9 PcxjuegavXaA48Ft2OhC4u4ww96FzlQTfiUZ9lced584fNdd5Q9BYcwDNRZ+iK954Lw1 HRHtR2vDDTZohSGIVj+iEBq6SP7PsjoNB2ylrKWULs+55WDWut9/OE7vNhLrJ458RvY7 ptIVJUiGeguGHqGmi4JBYEN8lSees2LZX8UQawwZztEP5ynEt4JS1n4E1OwKfbi8tXOn /kMw== X-Gm-Message-State: AOJu0Yx4JfJs56sdHhI7ehNQDixqa47HGCQnbmye8qEsvTY/RacGoQ/1 vUsMbk9C5IINwxfwPK9Kl39IRj0FK0bJgrATtmg= X-Google-Smtp-Source: AGHT+IHGTt921linEEXWDCp01ZjO817YGSh2tk5/R9L1I/NyyNv/Y620IaskzCyXzMF+0cM70k3Xtw== X-Received: by 2002:a17:906:1b4b:b0:9d3:ccf0:7617 with SMTP id p11-20020a1709061b4b00b009d3ccf07617mr3652071ejg.44.1698767720042; Tue, 31 Oct 2023 08:55:20 -0700 (PDT) Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com. [209.85.208.42]) by smtp.gmail.com with ESMTPSA id l9-20020a170906078900b009adc81bb544sm1176296ejc.106.2023.10.31.08.55.17 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 31 Oct 2023 08:55:18 -0700 (PDT) Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-51e24210395so17011a12.0 for ; Tue, 31 Oct 2023 08:55:17 -0700 (PDT) X-Received: by 2002:a50:fb04:0:b0:543:7dbc:243b with SMTP id d4-20020a50fb04000000b005437dbc243bmr73308edq.5.1698767716839; Tue, 31 Oct 2023 08:55:16 -0700 (PDT) MIME-Version: 1.0 References: <20231018181727.772-1-gurchetansingh@chromium.org> <20231018181727.772-2-gurchetansingh@chromium.org> In-Reply-To: From: Gurchetan Singh Date: Tue, 31 Oct 2023 08:55:04 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 2/2] drm/uapi: add explicit virtgpu context debug name To: Dmitry Osipenko Content-Type: multipart/alternative; boundary="0000000000005244bf0609052e81" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: josh.simonot@gmail.com, kraxel@redhat.com, dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" --0000000000005244bf0609052e81 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Oct 22, 2023 at 4:50=E2=80=AFPM Dmitry Osipenko < dmitry.osipenko@collabora.com> wrote: > 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 t= he > > string (excluding the terminator) **should** be <=3D 64 bytes. Otherwi= se, > > the debug_name will be truncated to 64 bytes. > > > > Link to open-source userspace: > > > https://android-review.googlesource.com/c/platform/hardware/google/gfxstr= eam/+/2787176 > > > > Signed-off-by: Gurchetan Singh > > Reviewed-by: Josh Simonot > > --- > > 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(-) > > Gerd, do you have objections to this UAPI change? > Bump. I say we wait another week and see if anyone cares [I suspect nobody does]. https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#= merge-criteria As per DRM guidelines, if there are no open comments and the change is reviewed, it is mergeable. > > -- > Best regards, > Dmitry > > --0000000000005244bf0609052e81 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sun, Oct 22, 2023 at 4:50=E2=80=AF= PM Dmitry Osipenko <dmi= try.osipenko@collabora.com> wrote:
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
>=C2=A0 =C2=A0 Linux kernel idiom (see PR_SET_NAME + PR_GET_NAME). Thoug= h,
>=C2=A0 =C2=A0 Android/FreeBSD get around this via setprogname(..)/getpr= ogname(..)
>=C2=A0 =C2=A0 in libc.
>
>=C2=A0 =C2=A0 On Android, names longer than 16 bytes are common.=C2=A0 = For example,
>=C2=A0 =C2=A0 one often encounters a program like "com.android.sys= temui".
>
>=C2=A0 =C2=A0 The virtio-gpu spec allows the debug name to be up to 64 = bytes, so
>=C2=A0 =C2=A0 ideally userspace should be able to set debug names up to= 64 bytes.
>
> 2) The current implementation determines the debug name using whatever=
>=C2=A0 =C2=A0 task initiated virtgpu.=C2=A0 This is could be a "Re= nderThread" of a
>=C2=A0 =C2=A0 larger program, when we actually want to propagate the de= bug name
>=C2=A0 =C2=A0 of the program.
>
> To fix these issues, add a new CONTEXT_INIT param that allows userspac= e
> 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 <=3D 64 bytes.=C2= =A0 Otherwise,
> the debug_name will be truncated to 64 bytes.
>
> Link to open-source userspace:
> https://a= ndroid-review.googlesource.com/c/platform/hardware/google/gfxstream/+/27871= 76
>
> Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> Reviewed-by: Josh Simonot <josh.simonot@gmail.com>
> ---
> Fixes suggested by Dmitry Osipenko
> v2:
>=C2=A0 =C2=A0 =C2=A0- Squash implementation and UAPI change into one co= mmit
>=C2=A0 =C2=A0 =C2=A0- Avoid unnecessary casts
>=C2=A0 =C2=A0 =C2=A0- Use bool when necessary
> v3:
>=C2=A0 =C2=A0 =C2=A0- Use DEBUG_NAME_MAX_LEN - 1 when copying string >
>=C2=A0 drivers/gpu/drm/virtio/virtgpu_drv.h=C2=A0 =C2=A0|=C2=A0 5 ++++<= br> >=C2=A0 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 39 ++++++++++++++++++++= ++----
>=C2=A0 include/uapi/drm/virtgpu_drm.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= |=C2=A0 2 ++
>=C2=A0 3 files changed, 40 insertions(+), 6 deletions(-)

Gerd, do you have objections to this UAPI change?

=
Bump.=C2=A0 I say we wait another week and see if anyone cares [= I suspect nobody does].


As per DRM guidelines, i= f there are no open comments and the change is reviewed, it is mergeable.
=C2=A0

--
Best regards,
Dmitry

--0000000000005244bf0609052e81--