On Thu, Oct 16, 2025 at 01:48:20PM -0700, Matthew Brost wrote:
Stop open coding pending job list in drivers. Add pending job list
iterator which safely walks DRM scheduler list asserting DRM scheduler
is stopped.
v2:
- Fix checkpatch (CI)
v3:
- Drop locked version (Christian)
Signed-off-by: Matthew Brost <[email protected]>
---
include/drm/gpu_scheduler.h | 52 +++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index fb88301b3c45..7f31eba3bd61 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -698,4 +698,56 @@ void drm_sched_entity_modify_sched(struct drm_sched_entity
*entity,
struct drm_gpu_scheduler **sched_list,
unsigned int num_sched_list);
+/* Inlines */
+
+/**
+ * struct drm_sched_pending_job_iter - DRM scheduler pending job iterator state
+ * @sched: DRM scheduler associated with pending job iterator
+ */
+struct drm_sched_pending_job_iter {
+ struct drm_gpu_scheduler *sched;
+};
+
+/* Drivers should never call this directly */
+static inline struct drm_sched_pending_job_iter
+__drm_sched_pending_job_iter_begin(struct drm_gpu_scheduler *sched)
+{
+ struct drm_sched_pending_job_iter iter = {
+ .sched = sched,
+ };
+
+ WARN_ON(!READ_ONCE(sched->pause_submit));
+ return iter;
+}
+
+/* Drivers should never call this directly */
+static inline void
+__drm_sched_pending_job_iter_end(const struct drm_sched_pending_job_iter iter)
+{
+ WARN_ON(!READ_ONCE(iter.sched->pause_submit));
+}
May be instead of these inline functions, we can add the code in a '({' block
in the below DEFINE_CLASS itself to avoid drivers from calling these inline
funcions? Though I agree these inline functions makes it cleaner to read.
+
+DEFINE_CLASS(drm_sched_pending_job_iter, struct drm_sched_pending_job_iter,
+ __drm_sched_pending_job_iter_end(_T),
+ __drm_sched_pending_job_iter_begin(__sched),
+ struct drm_gpu_scheduler *__sched);
+static inline void *
+class_drm_sched_pending_job_iter_lock_ptr(class_drm_sched_pending_job_iter_t
*_T)
+{ return _T; }
+#define class_drm_sched_pending_job_iter_is_conditional false
+
+/**
+ * drm_sched_for_each_pending_job() - Iterator for each pending job in
scheduler
+ * @__job: Current pending job being iterated over
+ * @__sched: DRM scheduler to iterate over pending jobs
+ * @__entity: DRM scheduler entity to filter jobs, NULL indicates no filter
+ *
+ * Iterator for each pending job in scheduler, filtering on an entity, and
+ * enforcing scheduler is fully stopped
+ */
+#define drm_sched_for_each_pending_job(__job, __sched, __entity)
\
+ scoped_guard(drm_sched_pending_job_iter, (__sched))
\
+ list_for_each_entry((__job), &(__sched)->pending_list, list)
\
+ for_each_if(!(__entity) || (__job)->entity ==
(__entity))
+
I am comparing it with DEFINE_CLASS usage in ttm driver here.
It looks like the body of this macro (where we call list_for_each_entry()),
doesn't use the drm_sched_pending_job_iter at all. So, looks like the only
reason we are using a DEFINE_CLASS with scoped_guard here is for those
WARN_ON() messages at the beginning and end of loop iteration, which is not
fully fool proof. Right?
I wonder if we really need DEFINE_CLASS here for that, though I am not
against using it.
Niranjana
#endif
--
2.34.1
