Hi Phillipp,
[…]
>
>>
>>> + // among all the fences. This can't become a UAF because
>>> each fence takes a
>>> + // reference of the fence context.
>>> + unsafe { bindings::dma_fence_init(slot, &Self::OPS,
>>> Opaque::cast_into(lock_ptr), context, seqno) };
>>> + }),
>>> + data <- data,
>>> + signalling: false,
>>> + signalling_cookie: false,
>>> + fctx: fctx,
>>> + });
>>> +
>>> + let b = KBox::pin_init(fence, GFP_KERNEL)?;
>>> +
>>> + // SAFETY: We don't move the contents of `b` anywhere here. After
>>> unwrapping it, ARef will
>>> + // take care of preventing memory moves.
>>> + let rawptr = KBox::into_raw(unsafe { Pin::into_inner_unchecked(b)
>>> });
>>> +
>>> + // SAFETY: `rawptr` was created validly above.
>>> + let aref = unsafe { ARef::from_raw(NonNull::new_unchecked(rawptr))
>>> };
>>> +
>>> + Ok(aref)
>>> + }
>>> +
>>> + /// Mark the beginning of a DmaFence signalling critical section.
>>> Should be called once a fence
>>> + /// gets published.
>>> + ///
>>> + /// The signalling critical section is marked as finished
>>> automatically once the fence signals.
>>> + pub fn begin_signalling(&mut self) {
>>> + // FIXME: this needs to be mutable, obviously, but we can't borrow
>>> mutably. *sigh*
>>
>> Is AtomicBool going away? Otherwise can you expand?
>
> The AtomicBool is just used in the example demo code.
>
> The issue here is that begin_signalling() should set a "this fence is
> currently in the signalling section"-flag. So the fence needs to be
> mutable. Then, however, Rust complains because self.signalling is not
> protected by any lock.
>
> So one needs some sort of synchronization. Stuffing a DmaFence into a
> SpinLock would be overkill, however, considering that the C code
> already takes care of properly taking all locks.
>
> I've asked about that problem on Zulip once:
> https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/.E2.9C.94.20ARef.20without.20locking/near/539747635
>
> Haven't looked deeper into solving it since, because those lockdep
> guards are kind of nice-to-have at the moment.
>
> I think the solution will be to make self.signalling an AtomicBool (I
> think you meant that above?)
Yes, that’s what I meant, i.e.: making self.signalling an AtomicBool.
