On Fri, 2024-09-13 at 17:05 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
> 
> Current kerneldoc for struct drm_sched_rq incompletely documents what
> fields are protected by the lock.
> 
> This is not good because it is misleading.
> 
> Lets fix it by listing all the elements which are protected by the
> lock.
> 
> While at it, lets also re-order the members so all protected by the
> lock
> are in a single group.
> 
> v2:
>  * Refer variables by kerneldoc syntax, more verbose commit text.
> (Philipp)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: Luben Tuikov <ltuiko...@gmail.com>
> Cc: Matthew Brost <matthew.br...@intel.com>
> Cc: Philipp Stanner <pstan...@redhat.com>
> Reviewed-by: Christian König <christian.koe...@amd.com>

Looks good, thx

Reviewed-by: Philipp Stanner <pstan...@redhat.com>


> ---
>  include/drm/gpu_scheduler.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/drm/gpu_scheduler.h
> b/include/drm/gpu_scheduler.h
> index 38465b78c7d5..2f58af00f792 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -243,10 +243,10 @@ struct drm_sched_entity {
>  /**
>   * struct drm_sched_rq - queue of entities to be scheduled.
>   *
> - * @lock: to modify the entities list.
>   * @sched: the scheduler to which this rq belongs to.
> - * @entities: list of the entities to be scheduled.
> + * @lock: protects @entities, @rb_tree_root and @current_entity.

nit: in case you'll provide a new version anyways you could consider
sorting these three to be congruent with the lines below.

Thank you!
P.


>   * @current_entity: the entity which is to be scheduled.
> + * @entities: list of the entities to be scheduled.
>   * @rb_tree_root: root of time based priory queue of entities for
> FIFO scheduling
>   *
>   * Run queue is a set of entities scheduling command submissions for
> @@ -254,10 +254,12 @@ struct drm_sched_entity {
>   * the next entity to emit commands from.
>   */
>  struct drm_sched_rq {
> -     spinlock_t                      lock;
>       struct drm_gpu_scheduler        *sched;
> -     struct list_head                entities;
> +
> +     spinlock_t                      lock;
> +     /* Following members are protected by the @lock: */
>       struct drm_sched_entity         *current_entity;
> +     struct list_head                entities;
>       struct rb_root_cached           rb_tree_root;
>  };
>  

Reply via email to