Re: [PATCH v2] rust: add trylock method support for lock backend

[Gary Guo <gary@garyguo.net>]

On Tue, 24 Sep 2024 00:02:46 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sun, Sep 15, 2024 at 12:23:39PM -0300, Filipe Xavier wrote:
> > Add a non-blocking trylock method to lock backend interface, mutex
> > and spinlock implementations. It includes a C helper for spin_trylock.
> > Rust Binder will use this method together with the new shrinker abstractions
> > to avoid deadlocks in the memory shrinker.
> >   
> 
> Getting better, thanks! Please see below:
> 
> > Link: https://lore.kernel.org/all/20240912-shrinker-v1-1-18b7f1253553@google.com
> > Signed-off-by: Filipe Xavier <felipe_life@live.com>
> > ---
> >  rust/helpers/spinlock.c           |  5 +++++
> >  rust/kernel/sync/lock.rs          | 16 ++++++++++++++++
> >  rust/kernel/sync/lock/mutex.rs    | 11 +++++++++++
> >  rust/kernel/sync/lock/spinlock.rs | 11 +++++++++++
> >  4 files changed, 43 insertions(+)
> > 
> > diff --git a/rust/helpers/spinlock.c b/rust/helpers/spinlock.c
> > index acc1376b833c..775ed4d549ae 100644
> > --- a/rust/helpers/spinlock.c
> > +++ b/rust/helpers/spinlock.c
> > @@ -22,3 +22,8 @@ void rust_helper_spin_unlock(spinlock_t *lock)
> >  {
> >  	spin_unlock(lock);
> >  }
> > +
> > +int rust_helper_spin_trylock(spinlock_t *lock)
> > +{
> > +	return spin_trylock(lock);
> > +}
> > diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> > index f6c34ca4d819..f4e51a5a1f23 100644
> > --- a/rust/kernel/sync/lock.rs
> > +++ b/rust/kernel/sync/lock.rs
> > @@ -58,6 +58,13 @@ unsafe fn init(
> >      #[must_use]
> >      unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState;
> >  
> > +    /// Tries to acquire the lock without blocking.  
> 
> As I suggested in v1, "without blocking" is not accurate here because
> a lock can be a spinlock. So you can just remove it. I think the word
> "Tries" itself implies "neither busy waiting nor blocking".

It seems that mutex_trylock contains a loop, so it's not
exactly wait-free.

Maybe `blocking` is indeed the correct word here?

Best,
Gary