On Tue, 2026-11-03 at 22:27 +0000, Younes Akhouayri wrote:
> On Tue, 3 Nov 2026 16:31:38 +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 {
> > + 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);
> > + }
>
> Hi Philip,
Hello,
>
> Is there a reason the credits are meant to stay consumed here, or should
> this branch do
>
> + *jq.as_mut().project().capacity += u64::from(job.cost);
>
> as well?
Nope, that's simply a bug. Well seen. This RFC has only seen basic
testing so far, because we still have to solve some quite ground laying
design questions (see cover-letter). Also the credit-count system was
re-added relatively recently.
Nevertheless, fixed this one locally, thanks for pointing it out
>
> > + last_completed: 0,
>
> FenceContext::new(initial_seqno, ...) hands out initial_seqno as the first
> fence seqno, and both the doc example and the tester pass 0. For that first
> job the check above becomes `0 >= 0`, so the worker signals the fence Ok
> right after run_job() returns, while the hardware is still working on it.
>
> Should last_completed start at initial_seqno.wrapping_sub(1), or is
> initial_seqno >= 1 an intended requirement that just needs documenting?
Typically the drivers will want to pass 1 as initial sequence number,
because they have the same problem you're describing, just on the
hardware: it's initialized to 0, so the first job would be successful
immediately. So they wanna start at 1.
Not sure how we should best address this.
@Daniel, maybe we can just remove `initial_seqno` and always start at
1? I'm not sure.
Thx for the review,
P.
