Re: [PATCH v2] rust: doc: Clarify safety invariants for Revocable type

["Benno Lossin" <lossin@kernel.org>]

On Fri May 23, 2025 at 10:55 AM CEST, Danilo Krummrich wrote:
> On Fri, May 23, 2025 at 10:42:58AM +0200, Benno Lossin wrote:
>> On Fri May 23, 2025 at 2:13 AM CEST, Marcelo Moreira wrote:
>> > 3. Clarified revoke_internal for SYNC = false and swap correction
>> > Proposed Documentation:
>> >  if self.is_available.swap(false, Ordering::Relaxed) {
>> >  if SYNC {
>> >                 // SAFETY: Just an FFI call, there are no further requirements.
>> >                 unsafe { bindings::synchronize_rcu() };
>> 
>> @Boqun: is this true?
>> 
>> If the answer is yes, then we should add this as a safe function in the
>> rcu module.
>
> I think it's a case for Klint, since synchronize_rcu() must not be called from
> atomic context, since it may block. Otherwise, there shouldn't be any additional
> requirements.

Makes sense, but then we should add this as a safe function (in a
separate patch). After all we already have other safe functions that
should be unsafe were it not for klint.

>> >  }  else {
>> >        // This branch for `revoke_nosync` requires the caller to prove
>> > that `data`
>> >        // can be dropped immediately without waiting for any RCU grace period.
>> 
>> I'm not sure that having a single function that does the revocation, but
>> has this going on is a good idea. The safety requirements will be pretty
>> complex.
>> 
>> @Danilo what do you think of inlining this function?
>
> Sure, if it makes documentation significantly easier, which seems to be the
> case, then it's probably worth.

Yeah then let's do it (also separate patch).

---
Cheers,
Benno