On 02/06/2021 19:51, Daniel Vetter wrote: > On Wed, Jun 02, 2021 at 03:06:50PM +0100, Steven Price wrote: >> On 21/05/2021 10:09, Daniel Vetter wrote: [...] >>> + if (!xa_empty(&job->deps)) >>> + return xa_erase(&job->deps, job->last_dep++); >> >> Rather than tracking last_dep separately this could be written using >> xa_find(): >> >> if (xa_find(&job->deps, &i, ULONG_MAX, XA_PRESENT)) >> return xa_erase(&job->deps, &i); > > I copypasted this from other drivers, imo consistency is better than > looking pretty. I think eventually we should stuff this as optional > helpers into drm/scheduler. > > Also yours walks the xa twice. Agreed this isn't as efficient (I was somewhat disappointed xarray doesn't expose a "get and remove the first element" API). Moving this into drm/scheduler seems like the right long term solution - so matching other drivers first is a good move. Thanks, Steve