On Wed, 2025-12-03 at 10:07 +0100, Philipp Stanner wrote: > +Cc Alex, Christian, Danilo > > > On Mon, 2025-12-01 at 10:39 -0800, Matthew Brost wrote: > >
[…] > > + > > +/** > > + * 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)) > > + > > #endif > > > See my comments in the first patch. The docu doesn't mention at all why > this new functionality exists and when and why users would be expected > to use it. > > As far as I remember from XDC, both AMD and Intel overwrite a timed out > jobs buffer data in the rings on GPU reset. To do so, the driver needs > the timedout job (passed through timedout_job() callback) and then > needs all the pending non-broken jobs. > > AFAICS your patch provides a generic iterator over the entire > pending_list. How is a driver then supposed to determine which are the > non-broken jobs (just asking, but that needs to be documented)? > > Could it make sense to use a different iterator which only returns jobs > of not belonging to the same context as the timedout-one? (forget about that comment, you do that with the entity-filter obviously) P.
