Re: [PATCH 7/7] libtrivfs: lock-less reference counting for trivfs_peropen objects

[Justus Winter]

Quoting Samuel Thibault (2014-05-23 01:40:19)
> Justus Winter, le Tue 13 May 2014 21:02:56 +0200, a écrit :
> > diff --git a/libtrivfs/protid-clean.c b/libtrivfs/protid-clean.c
> > index f98da6a..cce736d 100644
> > --- a/libtrivfs/protid-clean.c
> > +++ b/libtrivfs/protid-clean.c
> > @@ -31,19 +31,23 @@ trivfs_clean_protid (void *arg)
> >      (*trivfs_protid_destroy_hook) (cred);
> >  
> >    /* If we hold the only reference to the peropen, try to get rid of it. */
> > -  pthread_mutex_lock (&cntl->lock);
> > -  if (cred->po->refcnt == 1 && trivfs_peropen_destroy_hook)
> > +  if (refcount_deref (&cred->po->refcnt) == 0)
> >      {
> > -      pthread_mutex_unlock (&cntl->lock);
> > -      (*trivfs_peropen_destroy_hook) (cred->po);
> > -      pthread_mutex_lock (&cntl->lock);
> > +      if (trivfs_peropen_destroy_hook)
> > +        {
> > +          /* Reaquire a reference while we call the hook.  */
> > +          refcount_ref (&cred->po->refcnt);
> > +          (*trivfs_peropen_destroy_hook) (cred->po);
> > +          if (refcount_deref (&cred->po->refcnt) == 0)
> > +            goto free_po;
> > +        }
> > +      else
> > +        {
> > +        free_po:
> > +          ports_port_deref (cntl);
> > +          free (cred->po);
> > +        }
> 
> Why not doing it as before: check whether it's the last reference,
> instead of unreferencing, then re-referencing for the hook, and
> unreference again?

It is an optimization.  This way we can use just 1 atomic operation
when trivfs_peropen_destroy_hook is not used, and 3 when it is.  If we
do it like you described, we need two in any case.

A quick survey of the Hurd codebase revealed that trivfs-based
translators *not* using trivfs_peropen_destroy_hook are exec, pfinet
and pflocal.  Translators using it are storeio, term, fifo, hello*,
magic, mtab, new-fifo, streamio.

Justus