Re: [PATCH 7/8] rust: drm: Add JobQueue

[Younes Akhouayri]

On Fri, 6 Nov 2026 15:07:52 +0100, Philipp Stanner wrote:
> +
> +            drop(jq); // Release lock.
> +            T::run_job(work.fctx, runnable_job);
> +            jq = work.inner.lock(); // Retake lock.
> +
> +            // run_job() above needs to be called without lock protection. Thus,
> +            // it is possible that the driver instructed completion right before
> +            // we could re-take the lock.
> +            if jq.last_completed >= seqno {
> +                *jq.as_mut().project().capacity += u64::from(job.cost);
> +                fence.signal(Ok(()));
> +
> +                // Drop here directly if possible. Otherwise, let the DropWorker
> +                // take care of it.
> +                if core::mem::needs_drop::<T::JobDataType>() {
> +                    jq.drop_list.push_back(job);
> +                }
> +            } else {
> +                jq.running_jobs.push_back(job);
> +            }

When the IRQ wins the race, `complete_jobs_up_to_seqno()` kicks the DropWorker before the job lands on `drop_list`, and this branch pushes it there without kicking again. Am I reading it right that the job then sits on `drop_list` until the next completion or until the queue drops? If so, is that acceptable, or does this path need a `check_start_drop_worker()` too?


> +        self.with_inner(|inner| {
> +            let mut inner = inner.lock();
> +            let drop_list = core::mem::take(&mut inner.drop_list);
> +            let waiting_jobs = core::mem::take(&mut inner.waiting_jobs);
> +            let mut running_jobs = core::mem::take(&mut inner.running_jobs);
> +
> +            while let Some(job) = running_jobs.pop_front() {
> +                let fence = job.fence.as_ref().expect("fence not present");
> +                fence.signal(Err(self.err));
> +            }
> +
> +            drop(inner);
> +            drop(drop_list);
> +            drop(waiting_jobs);
> +            drop(running_jobs);
> +        })

In the `while let Some(job) = running_jobs.pop_front()` loop, `job` goes out of scope at the end of each iteration, while `inner` is still locked. If `JobDataType` has a `Drop` impl, that ends up in `DriverFence::drop` -> `synchronize_rcu()`; so is sleeping there with the irq spinlock held intended, or should those jobs be collected and dropped after `drop(inner)` like `drop_list` and `waiting_jobs` are?

Regards,
Younes