Re: [PATCH 6/7] libdiskfs: lock-less reference counting for peropen objects

[Samuel Thibault]

Justus Winter, le Tue 13 May 2014 21:02:55 +0200, a écrit :
> * libdiskfs/diskfs.h (struct peropen): Use refcount_t for field refcnt.
> * libdiskfs/peropen-make.c (diskfs_make_peropen): Initialize refcnt.
> * libdiskfs/peropen-rele.c (diskfs_release_peropen): Adjust accordingly.
> * libdiskfs/protid-make.c (diskfs_start_protid): Likewise.  Also, the
> node must no longer be locked, adjust comment accordingly.
> (diskfs_create_protid): Likewise.

Ack.

> ---
>  libdiskfs/diskfs.h       |  7 ++++---
>  libdiskfs/peropen-make.c |  2 +-
>  libdiskfs/peropen-rele.c | 21 ++++++++++-----------
>  libdiskfs/protid-make.c  |  8 ++++----
>  4 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/libdiskfs/diskfs.h b/libdiskfs/diskfs.h
> index 8151ddc..ae1a150 100644
> --- a/libdiskfs/diskfs.h
> +++ b/libdiskfs/diskfs.h
> @@ -28,6 +28,7 @@
>  #include <hurd/iohelp.h>
>  #include <idvec.h>
>  #include <features.h>
> +#include <refcount.h>
>  
>  #ifdef DISKFS_DEFINE_EXTERN_INLINE
>  #define DISKFS_EXTERN_INLINE
> @@ -57,7 +58,7 @@ struct peropen
>  {
>    int filepointer;
>    int lock_status;
> -  int refcnt;
> +  refcount_t refcnt;
>    int openstat;
>  
>    struct node *np;
> @@ -792,12 +793,12 @@ diskfs_create_node (struct node *dir, const char *name, 
> mode_t mode,
>                   struct dirstat *ds);
>  
>  /* Create and return a protid for an existing peropen PO in CRED,
> -   referring to user USER.  The node PO->np must be locked. */
> +   referring to user USER.  */
>  error_t diskfs_create_protid (struct peropen *po, struct iouser *user,
>                             struct protid **cred);
>  
>  /* Build and return in CRED a protid which has no user identification, for
> -   peropen PO.  The node PO->np must be locked.  */
> +   peropen PO.  */
>  error_t diskfs_start_protid (struct peropen *po, struct protid **cred);
>  
>  /* Finish building protid CRED started with diskfs_start_protid;
> diff --git a/libdiskfs/peropen-make.c b/libdiskfs/peropen-make.c
> index eba037f..6d5ca01 100644
> --- a/libdiskfs/peropen-make.c
> +++ b/libdiskfs/peropen-make.c
> @@ -31,7 +31,7 @@ diskfs_make_peropen (struct node *np, int flags, struct 
> peropen *context,
>  
>    po->filepointer = 0;
>    po->lock_status = LOCK_UN;
> -  po->refcnt = 0;
> +  refcount_init (&po->refcnt, 0);
>    po->openstat = flags;
>    po->np = np;
>    po->path = NULL;
> diff --git a/libdiskfs/peropen-rele.c b/libdiskfs/peropen-rele.c
> index d3f7492..877137b 100644
> --- a/libdiskfs/peropen-rele.c
> +++ b/libdiskfs/peropen-rele.c
> @@ -22,13 +22,8 @@
>  void
>  diskfs_release_peropen (struct peropen *po)
>  {
> -  pthread_mutex_lock (&po->np->lock);
> -
> -  if (--po->refcnt)
> -    {
> -      pthread_mutex_unlock (&po->np->lock);
> -      return;
> -    }
> +  if (refcount_deref (&po->refcnt) > 0)
> +    return;
>  
>    if (po->root_parent)
>      mach_port_deallocate (mach_task_self (), po->root_parent);
> @@ -40,10 +35,14 @@ diskfs_release_peropen (struct peropen *po)
>      mach_port_deallocate (mach_task_self (), po->shadow_root_parent);
>  
>    if (po->lock_status != LOCK_UN)
> -    fshelp_acquire_lock (&po->np->userlock, &po->lock_status,
> -                      &po->np->lock, LOCK_UN);
> -
> -  diskfs_nput (po->np);
> +    {
> +      pthread_mutex_lock (&po->np->lock);
> +      fshelp_acquire_lock (&po->np->userlock, &po->lock_status,
> +                           &po->np->lock, LOCK_UN);
> +      diskfs_nput (po->np);
> +    }
> +  else
> +    diskfs_nrele (po->np);
>  
>    free (po->path);
>    free (po);
> diff --git a/libdiskfs/protid-make.c b/libdiskfs/protid-make.c
> index b39b92a..22aaa2e 100644
> --- a/libdiskfs/protid-make.c
> +++ b/libdiskfs/protid-make.c
> @@ -20,7 +20,7 @@
>  #include <assert.h>
>  
>  /* Build and return in CRED a protid which has no user identification, for
> -   peropen PO.  The node PO->np must be locked.  */
> +   peropen PO.  */
>  error_t
>  diskfs_start_protid (struct peropen *po, struct protid **cred)
>  {
> @@ -29,7 +29,7 @@ diskfs_start_protid (struct peropen *po, struct protid 
> **cred)
>                                sizeof (struct protid), cred);
>    if (! err)
>      {
> -      po->refcnt++;
> +      refcount_ref (&po->refcnt);
>        (*cred)->po = po;
>        (*cred)->shared_object = MACH_PORT_NULL;
>        (*cred)->mapped = 0;
> @@ -55,8 +55,8 @@ diskfs_finish_protid (struct protid *cred, struct iouser 
> *user)
>    assert_perror (err);
>  }
>  
> -/* Create and return a protid for an existing peropen PO in CRED for USER.
> -   The node PO->np must be locked. */
> +/* Create and return a protid for an existing peropen PO in CRED for
> +   USER.  */
>  error_t
>  diskfs_create_protid (struct peropen *po, struct iouser *user,
>                     struct protid **cred)
> -- 
> 2.0.0.rc0
> 

-- 
Samuel
Hi ! I'm a .signature virus ! Copy me into your ~/.signature, please !