Received: by 2002:a05:6358:9144:b0:117:f937:c515 with SMTP id r4csp386691rwr; Thu, 27 Apr 2023 02:47:49 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5pJQ6ltBsdQkT3pYooPIYq71H81ZTjC+u5UsJCe/6ZxVxJIequcKkvSVVmUNdfVxdLEcry X-Received: by 2002:a05:6a20:1610:b0:f6:15f3:ca21 with SMTP id l16-20020a056a20161000b000f615f3ca21mr1151079pzj.3.1682588869069; Thu, 27 Apr 2023 02:47:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682588869; cv=none; d=google.com; s=arc-20160816; b=O+bNc+xPjIH4yNI0UsqL5gfh//6XwKXgqQiBAveknG0vXJst2xsHpM/d7fEd3h7C4W OTeLF6e0pD2KmVBNMH61YiCCiIrUgccqoXVhBsexhRy4Cn3nbg0OyYUI11N+RBIxMjkV a5DBcZwGbQ1+tjwj3d8ur8LMbHdBam6wzcxVhFfiEHBY9xEPMIq2BUGi14mtPGsKhAgD jf21nqMlpaPz4m1EOOS7N9WDICWrJYpvZk/kRNP36R0TC0KChHMZHpEsmvzl59AVG8Hc qcpaoSG1R/Lz3fkVzWeaU2DQBVnOBI8mZGaCBsrn9e1FHU0YdTX94n7e9DYu2/uLclvh Yb1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:from:date:dkim-signature; bh=ijOvNVXHQPQ7K3a8mfotlj3EqkLbm2iNVAzawaQHZIw=; b=AYRst3woCOSzZ+TwFUtDqhE9fr7qMqITYjIgii8U25bNH7yFGh5zM1cA3PyWkvGra4 xWmwpLtHQrPvZs8I2x7FQ5ZFQZLD9/lHKqzroQVwYxakMWTnovTai2naHBrx70VSldHf dvHSYxqe7GHDmfFsDkNkSJ5HR+Wwikbjnq1nTBVNfyZsNprG0YngIJ/RYqkuUGXZ0Bgl EV1QhZVXpRDcjUBEOoNj12me0Az0eYBCnPg6JaTYmzg0koLGSl7EEVhaglnmkaBAPzEa Q/QBeEbc9InWvMNYwsHIakI9kvx433VNVT2wWzTqZ0QpnT7uP5sraA5aiMBvXE/em4jD yPcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ffwll.ch header.s=google header.b=kFa3Dgvs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q36-20020a63f964000000b00512fab8c401si17976652pgk.427.2023.04.27.02.47.35; Thu, 27 Apr 2023 02:47:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@ffwll.ch header.s=google header.b=kFa3Dgvs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243094AbjD0Jj7 (ORCPT + 99 others); Thu, 27 Apr 2023 05:39:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56864 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243064AbjD0Jj5 (ORCPT ); Thu, 27 Apr 2023 05:39:57 -0400 Received: from mail-ej1-x633.google.com (mail-ej1-x633.google.com [IPv6:2a00:1450:4864:20::633]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 672624497 for ; Thu, 27 Apr 2023 02:39:55 -0700 (PDT) Received: by mail-ej1-x633.google.com with SMTP id a640c23a62f3a-95227e5164dso235505566b.1 for ; Thu, 27 Apr 2023 02:39:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; t=1682588394; x=1685180394; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:mail-followup-to:message-id:subject:cc:to :from:date:from:to:cc:subject:date:message-id:reply-to; bh=ijOvNVXHQPQ7K3a8mfotlj3EqkLbm2iNVAzawaQHZIw=; b=kFa3Dgvsdqo+BXHl6yahcNl1383pNFvNtKjCwWMfHTj+Bit3JIwCGJEQDI6N/QU00j 7X00IV28cyVTxMhXOHSSaNOtJlIO/f240AjyaV+PfAb4s7UETaAAtwNr4ZxvB9KF2UtB pZGfcxrFXB1KoRo0EnWbU8+HE3g7nM63d1OjQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682588394; x=1685180394; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:mail-followup-to:message-id:subject:cc:to :from:date:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ijOvNVXHQPQ7K3a8mfotlj3EqkLbm2iNVAzawaQHZIw=; b=JinA70+Nd+sx0CdGUKkjQy0wo/2REWyF7baWHqq3YClebNFnxH/tx/QdheDC2XhJ4G WOkSgQ7y1E60lmIiY3SlFe+DA3DMvEL02A/nquy26o0ApMnca20zjxIDy5F6Bzpuq68o 1m8Jz7LYxuyENSrtliyxXDXHx0EAMgaAuO8Pjn6As3aHaCKiKpmNcnWZC4tbeLF+Wr9Q rotEZlNUJWa4LDyzXL5RjEjsRtyQKnrEWOhnEh6X4+7vzxFTjkRO5ZzGyz/ODfFeyn9l 4kTKIu299gm6u+m+KiVHjxPCTxuXD/nJ+hf0JKGxHD+hvjnQtLoNj1aeMviBym6dYjOB aFAw== X-Gm-Message-State: AC+VfDzdgSJV683RS2zA6PJF5r6HTkpwDwszB82aUcm7mOuau06iM26K aolgYs31fXxfrcoPvMEB81RaHA== X-Received: by 2002:a17:906:7491:b0:95f:db5f:73b7 with SMTP id e17-20020a170906749100b0095fdb5f73b7mr888891ejl.0.1682588393848; Thu, 27 Apr 2023 02:39:53 -0700 (PDT) Received: from phenom.ffwll.local (212-51-149-33.fiber7.init7.net. [212.51.149.33]) by smtp.gmail.com with ESMTPSA id jt11-20020a170906ca0b00b00958434d4ecesm6820771ejb.13.2023.04.27.02.39.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Apr 2023 02:39:53 -0700 (PDT) Date: Thu, 27 Apr 2023 11:39:51 +0200 From: Daniel Vetter To: Rob Clark Cc: Emil Velikov , Rob Clark , Tvrtko Ursulin , Akhil P Oommen , Abhinav Kumar , dri-devel@lists.freedesktop.org, open list , Konrad Dybcio , "open list:DRM DRIVER FOR MSM ADRENO GPU" , Dmitry Baryshkov , "open list:DRM DRIVER FOR MSM ADRENO GPU" , Sean Paul Subject: Re: [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper Message-ID: Mail-Followup-To: Rob Clark , Emil Velikov , Rob Clark , Tvrtko Ursulin , Akhil P Oommen , Abhinav Kumar , dri-devel@lists.freedesktop.org, open list , Konrad Dybcio , "open list:DRM DRIVER FOR MSM ADRENO GPU" , Dmitry Baryshkov , "open list:DRM DRIVER FOR MSM ADRENO GPU" , Sean Paul References: <20230417201215.448099-1-robdclark@gmail.com> <20230417201215.448099-3-robdclark@gmail.com> <58977051-197b-f1f0-c795-9037e70d7a91@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Operating-System: Linux phenom 6.1.0-7-amd64 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 21, 2023 at 07:47:26AM -0700, Rob Clark wrote: > On Fri, Apr 21, 2023 at 2:33 AM Emil Velikov wrote: > > > > Greeting all, > > > > Sorry for the delay - Easter Holidays, food coma and all that :-) > > > > On Tue, 18 Apr 2023 at 15:31, Rob Clark wrote: > > > > > > On Tue, Apr 18, 2023 at 1:34 AM Daniel Vetter wrote: > > > > > > > > On Tue, Apr 18, 2023 at 09:27:49AM +0100, Tvrtko Ursulin wrote: > > > > > > > > > > On 17/04/2023 21:12, Rob Clark wrote: > > > > > > From: Rob Clark > > > > > > > > > > > > Make it work in terms of ctx so that it can be re-used for fdinfo. > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > --- > > > > > > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 ++-- > > > > > > drivers/gpu/drm/msm/msm_drv.c | 2 ++ > > > > > > drivers/gpu/drm/msm/msm_gpu.c | 13 ++++++------- > > > > > > drivers/gpu/drm/msm/msm_gpu.h | 12 ++++++++++-- > > > > > > drivers/gpu/drm/msm/msm_submitqueue.c | 1 + > > > > > > 5 files changed, 21 insertions(+), 11 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > > > > > > index bb38e728864d..43c4e1fea83f 100644 > > > > > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > > > > > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > > > > > > @@ -412,7 +412,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx, > > > > > > /* Ensure string is null terminated: */ > > > > > > str[len] = '\0'; > > > > > > - mutex_lock(&gpu->lock); > > > > > > + mutex_lock(&ctx->lock); > > > > > > if (param == MSM_PARAM_COMM) { > > > > > > paramp = &ctx->comm; > > > > > > @@ -423,7 +423,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx, > > > > > > kfree(*paramp); > > > > > > *paramp = str; > > > > > > - mutex_unlock(&gpu->lock); > > > > > > + mutex_unlock(&ctx->lock); > > > > > > return 0; > > > > > > } > > > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > > > > > index 3d73b98d6a9c..ca0e89e46e13 100644 > > > > > > --- a/drivers/gpu/drm/msm/msm_drv.c > > > > > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > > > > > @@ -581,6 +581,8 @@ static int context_init(struct drm_device *dev, struct drm_file *file) > > > > > > rwlock_init(&ctx->queuelock); > > > > > > kref_init(&ctx->ref); > > > > > > + ctx->pid = get_pid(task_pid(current)); > > > > > > > > > > Would it simplify things for msm if DRM core had an up to date file->pid as > > > > > proposed in > > > > > https://patchwork.freedesktop.org/patch/526752/?series=109902&rev=4 ? It > > > > > gets updated if ioctl issuer is different than fd opener and this being > > > > > context_init here reminded me of it. Maybe you wouldn't have to track the > > > > > pid in msm? > > > > > > The problem is that we also need this for gpu devcore dumps, which > > > could happen after the drm_file is closed. The ctx can outlive the > > > file. > > > > > I think we all kept forgetting about that. MSM had support for ages, > > while AMDGPU is the second driver to land support - just a release > > ago. > > > > > But the ctx->pid has the same problem as the existing file->pid when > > > it comes to Xorg.. hopefully over time that problem just goes away. > > > > Out of curiosity: what do you mean with "when it comes to Xorg" - the > > "was_master" handling or something else? > > The problem is that Xorg is the one to open the drm fd, and then > passes the fd to the client.. so the pid of drm_file is the Xorg pid, > not the client. Making it not terribly informative. > > Tvrtko's patch he linked above would address that for drm_file, but > not for other driver internal usages. Maybe it could be wired up as a > helper so that drivers don't have to re-invent that dance. Idk, I > have to think about it. > > Btw, with my WIP drm sched fence signalling patch lockdep is unhappy > when gpu devcore dumps are triggered. I'm still pondering how to > decouple the locking so that anything coming from fs (ie. > show_fdinfo()) is decoupled from anything that happens in the fence > signaling path. But will repost this series once I get that sorted > out. So the cleanest imo is that you push most of the capturing into a worker that's entirely decoupled. If you have terminal context (i.e. on first hang they stop all further cmd submission, which is anyway what vk/arb_robustness want), then you don't have to capture at tdr time, because there's no subsequent batch that will wreck the state. But it only works if your gpu ctx don't have recoverable semantics. If you can't do that it's a _lot_ of GFP_ATOMIC and trylock and bailing out if any fails :-/ -Daniel > > BR, > -R > > > > > > guess I could do a similar dance to your patch to update the pid > > > whenever (for ex) a submitqueue is created. > > > > > > > Can we go one step further and let the drm fdinfo stuff print these new > > > > additions? Consistency across drivers and all that. > > > > > > Hmm, I guess I could _also_ store the overridden comm/cmdline in > > > drm_file. I still need to track it in ctx (msm_file_private) because > > > I could need it after the file is closed. > > > > > > Maybe it could be useful to have a gl extension to let the app set a > > > name on the context so that this is useful beyond native-ctx (ie. > > > maybe it would be nice to see that "chrome: lwn.net" is using less gpu > > > memory than "chrome: phoronix.com", etc) > > > > > > > /me awaits for the series to hit the respective websites ;-) > > > > But seriously - the series from Tvrtko (thanks for the link, will > > check in a moment) makes sense. Although given the livespan issue > > mentioned above, I don't think it's applicable here. > > > > So if it were me, I would consider the two orthogonal for the > > short/mid term. Fwiw this and patch 1/3 are: > > Reviewed-by: Emil Velikov > > > > HTH > > -Emil -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch