Re: [TECH TOPIC] Rust for Linux

[Greg KH <greg@kroah.com>]

On Wed, Jul 07, 2021 at 05:15:01PM +0200, Miguel Ojeda wrote:
> For instance, we have a `Ref` type that is similar to `Arc` but reuses
> the `refcount_t` from C and introduces saturation instead of aborting
> [3]
> 
> [3] https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/sync/arc.rs

This is interesting code in that I think you are missing the part where
you still need a lock on the object to prevent one thread from grabbing
a reference while another one is dropping the last reference.  Or am I
missing something?

The code here:

   fn drop(&mut self) {
        // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
        // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
        // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
        // freed/invalid memory as long as it is never dereferenced.
        let refcount = unsafe { self.ptr.as_ref() }.refcount.get();

        // INVARIANT: If the refcount reaches zero, there are no other instances of `Ref`, and
        // this instance is being dropped, so the broken invariant is not observable.
        // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
        let is_zero = unsafe { rust_helper_refcount_dec_and_test(refcount) };
        if is_zero {
            // The count reached zero, we must free the memory.
            //
            // SAFETY: The pointer was initialised from the result of `Box::leak`.
            unsafe { Box::from_raw(self.ptr.as_ptr()) };
        }
   }

Has a lot of interesting comments, and maybe just because I know nothing
about Rust, but why on the first line of the comment is there always
guaranteed a reference to the object at this point in time?  And yes
it's ok to have a pointer to memory that is not dereferenced, but is
that what is happening here?

I feel you are trying to duplicate the logic of a "struct kref" here,
and that requires a lock to work properly.  Where is the lock here?

thanks,

greg k-h