linux-kernel - Re: [PATCH v2] drm/sched: drm_sched_job

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5d645ef9fa26c0de82135755d4fa573ec409e3ef.camel@mailbox.org>
Date: Wed, 05 Mar 2025 14:17:15 +0100
From: Philipp Stanner <phasta@...lbox.org>
To: Philipp Stanner <phasta@...nel.org>, Matthew Brost
 <matthew.brost@...el.com>,  Danilo Krummrich <dakr@...nel.org>, Christian
 König <ckoenig.leichtzumerken@...il.com>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>,  Thomas Zimmermann <tzimmermann@...e.de>, David
 Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Tvrtko Ursulin
 <tvrtko.ursulin@...lia.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/sched: drm_sched_job_cleanup(): correct false doc

On Tue, 2025-03-04 at 15:13 +0100, Philipp Stanner wrote:
> drm_sched_job_cleanup()'s documentation claims that calling
> drm_sched_job_arm() is a "point of no return", implying that
> afterwards
> a job cannot be cancelled anymore.
> 
> This is not correct, as proven by the function's code itself, which
> takes a previous call to drm_sched_job_arm() into account. In truth,
> the
> decisive factors are whether fences have been shared (e.g., with
> other
> processes) and if the job has been submitted to an entity already.
> 
> Correct the wrong docstring.
> 
> Signed-off-by: Philipp Stanner <phasta@...nel.org>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@...lia.com>

Applied to drm-misc-next.

P.

> ---
> Changes in v2:
>   - Also adjust comment about arm inside function's body. (Tvrtko)
> ---
>  drivers/gpu/drm/scheduler/sched_main.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index c634993f1346..7e71e89cea89 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -1013,11 +1013,13 @@ EXPORT_SYMBOL(drm_sched_job_has_dependency);
>   * Cleans up the resources allocated with drm_sched_job_init().
>   *
>   * Drivers should call this from their error unwind code if @job is
> aborted
> - * before drm_sched_job_arm() is called.
> + * before it was submitted to an entity with
> drm_sched_entity_push_job().
>   *
> - * After that point of no return @job is committed to be executed by
> the
> - * scheduler, and this function should be called from the
> - * &drm_sched_backend_ops.free_job callback.
> + * Since calling drm_sched_job_arm() causes the job's fences to be
> initialized,
> + * it is up to the driver to ensure that fences that were exposed to
> external
> + * parties get signaled. drm_sched_job_cleanup() does not ensure
> this.
> + *
> + * This function must also be called in &struct
> drm_sched_backend_ops.free_job
>   */
>  void drm_sched_job_cleanup(struct drm_sched_job *job)
>  {
> @@ -1028,7 +1030,7 @@ void drm_sched_job_cleanup(struct drm_sched_job
> *job)
>  		/* drm_sched_job_arm() has been called */
>  		dma_fence_put(&job->s_fence->finished);
>  	} else {
> -		/* aborted job before committing to run it */
> +		/* aborted job before arming */
>  		drm_sched_fence_free(job->s_fence);
>  	}
>