Re: [RFC PATCH v5 0/6] rust: Add drm::JobQueue

[Daniel Almeida <daniel.almeida@collabora.com>]

Hi Philipp,

> On 9 Oct 2026, at 16:11, Philipp Stanner <phasta@kernel.org> wrote:
> 
> Changes in v5:
>  - Add back the Revocable<JobQueueInner…> from my earliest RFCs. Reason
>    is still the same: JQ dropping while dependency fences signal could
>    deadlock. Similarly, this solves a dependency kicking of the work
>    item again while drop() is running.

Can you expand a bit on this? IIUC the problem is introduced by trying to take
the jq lock on signal? If so, this could be avoided by queuing the worker from
that path instead?

>  - Do not call run_job() with a lock held, since run_job() might sleep.
>    AFAICS, this also makes the proposed XArray solution impossible and
>    demands that we use a list.

Can you expand on this a bit?

>  - Re-add the earlier credit-count-system, so that run_job() is
>    infallible. Simplifies the design quite a bit. (Danilo)

What is AGX’s position on this? IIRC, this credit system could not represent
their submission model exactly, like it can for others.


>  - Drop jobs in a deferred manner through the work item so that job
>    payload data can do atomic-hostile stuff.
>  - Because of the point above, I suggest that we stop dropping a
>    DriverFence on signal (Danilo's and my idea). It has no real
>    advantage, and getting rid of it allows for better controlling what
>    drops when through JobQueue. For fence it's only relevant that there
>    is an RCU grace period, thus:
>  - Replace DriverFence::drop()'s call_rcu() with synchronize_rcu(). We
>    would, therefore, demand that everyone who's got a problem with the
>    delay drops via work item.

I will get back to you.

> 
> This can still not be tested as I'm blocked by pin-init vs self-ref.
> Just FYI.
> 
> I would appreciate confirmation of these following thoughts on locking
> design:
> 
> 1.
> I believe that callers of jq.new_job() and jq.submit_job() do not need
> an outer serialization mutex. Reason is that JQ serializes with its
> internal lock. Sequence numbers cannot take over older seqnos because it
> is submit_job() who sets the seqno.

I guess this makes sense, so long as submit_job() doesn’t take &mut self?

Also, submit_job() assigning seqnos is not compatible with the XArray IIRC.
Fine if going with lists, I suppose.


> 
> 2.
> I, furthermore, believe that a driver will not need to hold a
> driver-lock while calling jq.complete_jobs_up_to_seqno().

In tyr, we hold _no_ locks. The driver signals the fences, and this will
enqueue the worker thread.

> 
> IOW, I am suggesting that a driver can always safely use JQ without an
> outer lock, and actually *should* do so to avoid potential locking
> issues.
> 
> Reason:
> 
> let driver_stuff = foo.lock();
> let current_gpu_seqno = driver_stuff.get_seqno();
> drop(driver_stuff);
> jq.complete_jobs_up_to_seqno(current_gpu_seqno);
> 
> Thus, I believe that it is OK for me to take the JQ lock in
> jq.complete_jobs_up_to_seqno().
> 
> Please correct me if I'm missing something.
> 
> Should I be wrong and a lock inversion could occur, then we could
> hypothetically add a fence Signaler object, with which we could signal
> fences with a different lock. Similar to how drm_sched does it with the
> hardware_fence.
> 
> 
> Regards
> P.
> 
> 

I will have a more thorough look this week :)

— Daniel