On Thu, Feb 06, 2025 at 04:40:31PM +0000, Tvrtko Ursulin wrote:
> We can re-order some struct members and take u32 credits outside of the
> pointer sandwich and also for the last_dependency member we can get away
> with an unsigned int since for dependency we use xa_limit_32b.
>
> Pahole report before:
> /* size: 160, cachelines: 3, members: 14 */
> /* sum members: 156, holes: 1, sum holes: 4 */
> /* last cacheline: 32 bytes */
>
> And after:
> /* size: 152, cachelines: 3, members: 14 */
> /* last cacheline: 24 bytes */
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Danilo Krummrich <d...@kernel.org>
> Cc: Matthew Brost <matthew.br...@intel.com>
> Cc: Philipp Stanner <pha...@kernel.org>
> ---
> include/drm/gpu_scheduler.h | 38 +++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index a0ff08123f07..68da3dec8dba 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -338,8 +338,14 @@ struct drm_sched_fence *to_drm_sched_fence(struct
> dma_fence *f);
> * to schedule the job.
> */
> struct drm_sched_job {
> - struct spsc_node queue_node;
> - struct list_head list;
> + u64 id;
> +
> + /**
> + * @submit_ts:
> + *
> + * When the job was pushed into the entity queue.
> + */
> + ktime_t submit_ts;
>
> /**
> * @sched:
> @@ -349,24 +355,30 @@ struct drm_sched_job {
> * has finished.
> */
> struct drm_gpu_scheduler *sched;
> +
> struct drm_sched_fence *s_fence;
> + struct drm_sched_entity *entity;
>
> + enum drm_sched_priority s_priority;
> u32 credits;
> + /** @last_dependency: tracks @dependencies as they signal */
> + unsigned int last_dependency;
> + atomic_t karma;
> +
> + struct spsc_node queue_node;
> + struct list_head list;
>
> /*
> * work is used only after finish_cb has been used and will not be
> * accessed anymore.
> */
> union {
> - struct dma_fence_cb finish_cb;
> - struct work_struct work;
> + struct dma_fence_cb finish_cb;
> + struct work_struct work;
I usually prefer to leave those things alone, but since you change most of this
struct anyways...
With or without this diff:
Acked-by: Danilo Krummrich <d...@kernel.org>
> };
>
> - uint64_t id;
> - atomic_t karma;
> - enum drm_sched_priority s_priority;
> - struct drm_sched_entity *entity;
> struct dma_fence_cb cb;
> +
> /**
> * @dependencies:
> *
> @@ -375,16 +387,6 @@ struct drm_sched_job {
> * drm_sched_job_add_implicit_dependencies().
> */
> struct xarray dependencies;
> -
> - /** @last_dependency: tracks @dependencies as they signal */
> - unsigned long last_dependency;
> -
> - /**
> - * @submit_ts:
> - *
> - * When the job was pushed into the entity queue.
> - */
> - ktime_t submit_ts;
> };
>
> static inline bool drm_sched_invalidate_job(struct drm_sched_job *s_job,
> --
> 2.48.0
>
