>
>>> +}
>>> +// Necessary to guarantee that `inner` always comes first and can be freed
>>> by C.
>>> +// Also useful for using casts instead of container_of().
>>> +#[repr(C)]
>>> +#[pin_data]
>>> +struct DriverFenceData<'a, T: Send + Sync + FenceCtxOps> {
>>> + #[pin]
>>> + /// The inner fence.
>>> + // Must always be the first member so that unsafe casting works; but
>>> also
>>> + // necessary so that the C backend can free the allocation (coming
>>> from our
>>> + // Rust code) with kfree_rcu().
>>> + inner: Fence,
>>> + /// Callback head for dropping this in a deferred manner through RCU.
>>> + rcu_head: bindings::callback_head,
>>> + /// Reference to access the FenceCtx. Useful for obtaining name
>>> parameters.
>>> + fctx: &'a FenceCtx<T>,
>>
>> This creates a self-referential borrow in the JobQueue, since it holds both
>> (a) the
>> context to mint new seqnos from and (b) the xarray with fences which borrow
>> from
>> that same context. This issue does not exist with the Arc and we should
>> really
>> fix it before moving with this series.
>
> Can you detail that a bit more. So your JobQueue version has a `data:
> T` where T holds both the xarray and a jobqueue. The xarray contains
> fences that hold references to the FenceCtx. But shouldn't that be fine
> since the life time ensures that the drop order is correct?
>
>
> btw we are recently coming up with a proposal on how JobQueue could
> store DriverFences for the driver. I want to present some RFC for that
> soonish.
>
Let me address this first, I will reply to your other points later today :)
So about “your JobQueue version has a data: T where T holds both the xarray
and a jobqueue”: no. The JobQueue has an xarray backing all the stages and
a context to mint new seqnos from:
struct JobQueueInner {
fifo: XArray<XaEntry>
fctx: FenceContext
// other fields..
}
This fifo backs all the stages in the queue, and we track stages by tracking
indices pointing to the XArray.
XaEntry is split between a fallible prepare() and a infallible commit():
// prepare() stores this:
XaEntry::Reserved {
job: Arc<T::Job>,
uninit_fence: DriverFenceAllocation<???, ...>,
deps: KVec<ARef<Fence>>,
// other fields
}
XaEntry::Live is similar, except that the allocation is initialized and there's
the whole DriverFence/Fence split and etc.
Under your proposed design, fences must borrow their context from somewhere,
and this somewhere is a sibling field in the JobQueue. This relationship is
unrepresentable in Rust. It only works if you don't store these two things
together.
There are probably a few different ways of fixing this, but I recommend going
back to refcounting.
— Daniel
