> On 3 Jun 2026, at 14:14, Boris Brezillon <[email protected]>
> wrote:
>
> On Wed, 3 Jun 2026 13:41:02 -0300
> Daniel Almeida <[email protected]> wrote:
>
>>> + /// Called when the fence is signaled.
>>> + ///
>>> + /// This is called from the fence signaling path, which may be in
>>> interrupt
>>> + /// context or with locks held, which is why `self` is only borrowed,
>>> so that
>>> + /// it cannot drop. Implementations must not sleep or perform
>>> + /// long-running operations.
>>> + ///
>>> + /// An implementation likely wants to inform itself (e.g., through a
>>> work item)
>>> + /// within this callback that the associated [`FenceCbRegistration`]
>>> can now be
>>> + /// dropped.
>>> + fn called(&mut self);
>>
>> This is a central point. We ideally would want this to consume self, because
>> we
>> may want to move things out of the callback.
>
> This one comes from me. The rationale being that ::called() is called
> from an atomic context, and the resources attached to the callback data
> might require acquiring other sleeping locks to be released, and
> sometimes you don't even notice immediately because said resources are
> refcounted, and the lock is only acquired when you happen to be the
> last owner. Yes, those can be caught at runtime if the C side is
> properly annotated with might_sleep(), but that's not always the case.
>
> If we defer the drop of the data only when the FenceCb is
> dropped/recycled, we're at least not constrained by this "runs in
> atomic context" thing.
>
This design does not solve it, because one can quite trivially get around this
restriction using Option<T> as I said. If your point is “don’t run any drop()
here”,
then &mut self doesn’t do it.
>>
>> Consider a fence design where signal() consumes self. Now consider this:
>>
>> ```
>> impl FenceCb for MyCallback {
>> fn called(&mut self) {
>> // Can't move the fence out, so we have to put an Option<T> just to be able
>> // to move.
>> if let Some(f) = self.some_fence.take() {
>> f.signal();
>> }
>> }
>> ```
>>
>> This used to be the case when our version of the job queue used the "proxy
>> fence" design:
>>
>>
>> ```
>> // Callback on the hw fence
>> impl FenceCb for MyCallback {
>> fn called(&mut self) {
>> if let Some(f) = self.submit_fence.take() {
>> f.signal();
>> }
>
> I'm pretty sure lockdep won't like it anyway, because this is nested
> locking of the same lock class. For such proxies, we'll need to teach
> lockdep about the nesting like has been recently done on
> dma_fence_array & co. But I'm digressing.
Yeah, but this is more about resource transfer in general, not
this pattern specifically.
I agree that this has issues, and yes, lockdep complained back
then :)
>
>> }
>> ```
>>
>> Although this is not the case anymore, since we phased out this design given
>> Christian's recent work. Still, we should ideally not require Option<T> here
>> in
>> general just to make resource transfer possible.
>
> I see. OTOH, don't we need to make this inner data movable if we want
> to cancel the FenceCb before the fence is signaled anyway? And that's
> most certainly a case we have in the teardown path.
Can you expand a bit on what you mean here?
