| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 18 Apr 2023 09:27:49 +0100
From: Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>
To: Rob Clark <robdclark@...il.com>, dri-devel@...ts.freedesktop.org
Cc: Rob Clark <robdclark@...omium.org>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Sean Paul <sean@...rly.run>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Akhil P Oommen <quic_akhilpo@...cinc.com>,
Chia-I Wu <olvaffe@...il.com>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<linux-arm-msm@...r.kernel.org>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<freedreno@...ts.freedesktop.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper
On 17/04/2023 21:12, Rob Clark wrote:
> From: Rob Clark <robdclark@...omium.org>
>
> Make it work in terms of ctx so that it can be re-used for fdinfo.
>
> Signed-off-by: Rob Clark <robdclark@...omium.org>
> ---
> 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?
Regards,
Tvrtko
> + mutex_init(&ctx->lock);
> msm_submitqueue_init(dev, ctx);
>
> ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, current);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index c403912d13ab..f0f4f845c32d 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -327,18 +327,17 @@ find_submit(struct msm_ringbuffer *ring, uint32_t fence)
>
> static void retire_submits(struct msm_gpu *gpu);
>
> -static void get_comm_cmdline(struct msm_gem_submit *submit, char **comm, char **cmd)
> +static void get_comm_cmdline(struct msm_file_private *ctx, char **comm, char **cmd)
> {
> - struct msm_file_private *ctx = submit->queue->ctx;
> struct task_struct *task;
>
> - WARN_ON(!mutex_is_locked(&submit->gpu->lock));
> -
> /* Note that kstrdup will return NULL if argument is NULL: */
> + mutex_lock(&ctx->lock);
> *comm = kstrdup(ctx->comm, GFP_KERNEL);
> *cmd = kstrdup(ctx->cmdline, GFP_KERNEL);
> + mutex_unlock(&ctx->lock);
>
> - task = get_pid_task(submit->pid, PIDTYPE_PID);
> + task = get_pid_task(ctx->pid, PIDTYPE_PID);
> if (!task)
> return;
>
> @@ -372,7 +371,7 @@ static void recover_worker(struct kthread_work *work)
> if (submit->aspace)
> submit->aspace->faults++;
>
> - get_comm_cmdline(submit, &comm, &cmd);
> + get_comm_cmdline(submit->queue->ctx, &comm, &cmd);
>
> if (comm && cmd) {
> DRM_DEV_ERROR(dev->dev, "%s: offending task: %s (%s)\n",
> @@ -460,7 +459,7 @@ static void fault_worker(struct kthread_work *work)
> goto resume_smmu;
>
> if (submit) {
> - get_comm_cmdline(submit, &comm, &cmd);
> + get_comm_cmdline(submit->queue->ctx, &comm, &cmd);
>
> /*
> * When we get GPU iova faults, we can get 1000s of them,
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 7a4fa1b8655b..b2023a42116b 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -377,17 +377,25 @@ struct msm_file_private {
> */
> int sysprof;
>
> + /** @pid: Process that opened this file. */
> + struct pid *pid;
> +
> + /**
> + * lock: Protects comm and cmdline
> + */
> + struct mutex lock;
> +
> /**
> * comm: Overridden task comm, see MSM_PARAM_COMM
> *
> - * Accessed under msm_gpu::lock
> + * Accessed under msm_file_private::lock
> */
> char *comm;
>
> /**
> * cmdline: Overridden task cmdline, see MSM_PARAM_CMDLINE
> *
> - * Accessed under msm_gpu::lock
> + * Accessed under msm_file_private::lock
> */
> char *cmdline;
>
> diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
> index 0e803125a325..0444ba04fa06 100644
> --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
> @@ -61,6 +61,7 @@ void __msm_file_private_destroy(struct kref *kref)
> }
>
> msm_gem_address_space_put(ctx->aspace);
> + put_pid(ctx->pid);
> kfree(ctx->comm);
> kfree(ctx->cmdline);
> kfree(ctx);
Powered by blists - more mailing lists