bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] libports: lock-less reference counting for port_info obj


From: Samuel Thibault
Subject: Re: [PATCH 2/2] libports: lock-less reference counting for port_info objects
Date: Sun, 5 Oct 2014 21:04:47 +0200
User-agent: Mutt/1.5.21+34 (58baf7c9f32f) (2010-12-30)

Ack, thanks!


Justus Winter, le Fri 05 Sep 2014 17:57:22 +0200, a écrit :
> * libports/ports.h (struct port_info): Use the new type.
> * libports/lookup-port.c: No need to lock _ports_lock anymore.
> * libports/bucket-iterate.c: Likewise.
> * libports/complete-deallocate.c: Check if someone reacquired a
> reference through a hash table lookup.
> * libports/create-internal.c: Use the new reference counting primitives.
> * libports/get-right.c: Likewise.
> * libports/import-port.c: Likewise.
> * libports/port-deref-weak.c: Likewise.
> * libports/port-deref.c: Likewise.
> * libports/port-ref-weak.c: Likewise.
> * libports/port-ref.c: Likewise.
> * libports/reallocate-from-external.c: Likewise.
> * libports/transfer-right.c: Likewise.
> * utils/rpctrace.c: Likewise.
> ---
>  libports/bucket-iterate.c           |  4 +---
>  libports/complete-deallocate.c      | 14 ++++++++++++++
>  libports/create-internal.c          |  3 +--
>  libports/get-right.c                |  2 +-
>  libports/import-port.c              |  3 +--
>  libports/lookup-port.c              |  4 +---
>  libports/port-deref-weak.c          | 10 +++-------
>  libports/port-deref.c               | 34 ++++++++++++++++------------------
>  libports/port-ref-weak.c            |  6 +-----
>  libports/port-ref.c                 |  6 +-----
>  libports/ports.h                    |  4 ++--
>  libports/reallocate-from-external.c |  2 +-
>  libports/transfer-right.c           |  2 +-
>  utils/rpctrace.c                    | 10 ++++++++--
>  14 files changed, 52 insertions(+), 52 deletions(-)
> 
> diff --git a/libports/bucket-iterate.c b/libports/bucket-iterate.c
> index 79b6d72..b021b99 100644
> --- a/libports/bucket-iterate.c
> +++ b/libports/bucket-iterate.c
> @@ -35,7 +35,6 @@ _ports_bucket_class_iterate (struct hurd_ihash *ht,
>    size_t i, n, nr_items;
>    error_t err;
>  
> -  pthread_mutex_lock (&_ports_lock);
>    pthread_rwlock_rdlock (&_ports_htable_lock);
>  
>    if (ht->nr_items == 0)
> @@ -59,13 +58,12 @@ _ports_bucket_class_iterate (struct hurd_ihash *ht,
>  
>        if (class == 0 || pi->class == class)
>       {
> -       pi->refcnt++;
> +       refcounts_ref (&pi->refcounts, NULL);
>         p[n] = pi;
>         n++;
>       }
>      }
>    pthread_rwlock_unlock (&_ports_htable_lock);
> -  pthread_mutex_unlock (&_ports_lock);
>  
>    if (n != 0 && n != nr_items)
>      {
> diff --git a/libports/complete-deallocate.c b/libports/complete-deallocate.c
> index 4768dab..0d852f5 100644
> --- a/libports/complete-deallocate.c
> +++ b/libports/complete-deallocate.c
> @@ -29,15 +29,29 @@ _ports_complete_deallocate (struct port_info *pi)
>  
>    if (pi->port_right)
>      {
> +      struct references result;
> +
>        pthread_rwlock_wrlock (&_ports_htable_lock);
> +      refcounts_references (&pi->refcounts, &result);
> +      if (result.hard > 0 || result.weak > 0)
> +        {
> +          /* A reference was reacquired through a hash table lookup.
> +             It's fine, we didn't touch anything yet. */
> +          pthread_mutex_unlock (&_ports_htable_lock);
> +          return;
> +        }
> +
>        hurd_ihash_locp_remove (&_ports_htable, pi->ports_htable_entry);
>        hurd_ihash_locp_remove (&pi->bucket->htable, pi->hentry);
>        pthread_rwlock_unlock (&_ports_htable_lock);
> +
>        mach_port_mod_refs (mach_task_self (), pi->port_right,
>                         MACH_PORT_RIGHT_RECEIVE, -1);
>        pi->port_right = MACH_PORT_NULL;
>      }
>  
> +  pthread_mutex_lock (&_ports_lock);
> +
>    pi->bucket->count--;
>    pi->class->count--;
>  
> diff --git a/libports/create-internal.c b/libports/create-internal.c
> index 8543986..2d85931 100644
> --- a/libports/create-internal.c
> +++ b/libports/create-internal.c
> @@ -54,8 +54,7 @@ _ports_create_port_internal (struct port_class *class,
>      }
>  
>    pi->class = class;
> -  pi->refcnt = 1;
> -  pi->weakrefcnt = 0;
> +  refcounts_init (&pi->refcounts, 1, 0);
>    pi->cancel_threshold = 0;
>    pi->mscount = 0;
>    pi->flags = 0;
> diff --git a/libports/get-right.c b/libports/get-right.c
> index 89050c6..8681f46 100644
> --- a/libports/get-right.c
> +++ b/libports/get-right.c
> @@ -41,7 +41,7 @@ ports_get_right (void *port)
>    if ((pi->flags & PORT_HAS_SENDRIGHTS) == 0)
>      {
>        pi->flags |= PORT_HAS_SENDRIGHTS;
> -      pi->refcnt++;
> +      refcounts_ref (&pi->refcounts, NULL);
>        err = mach_port_request_notification (mach_task_self (),
>                                           pi->port_right,
>                                           MACH_NOTIFY_NO_SENDERS,
> diff --git a/libports/import-port.c b/libports/import-port.c
> index 2660672..c337c85 100644
> --- a/libports/import-port.c
> +++ b/libports/import-port.c
> @@ -48,8 +48,7 @@ ports_import_port (struct port_class *class, struct 
> port_bucket *bucket,
>      return ENOMEM;
>    
>    pi->class = class;
> -  pi->refcnt = 1 + !!stat.mps_srights;
> -  pi->weakrefcnt = 0;
> +  refcounts_init (&pi->refcounts, 1 + !!stat.mps_srights, 0);
>    pi->cancel_threshold = 0;
>    pi->mscount = stat.mps_mscount;
>    pi->flags = stat.mps_srights ? PORT_HAS_SENDRIGHTS : 0;
> diff --git a/libports/lookup-port.c b/libports/lookup-port.c
> index 858ee11..2f0eee9 100644
> --- a/libports/lookup-port.c
> +++ b/libports/lookup-port.c
> @@ -28,7 +28,6 @@ ports_lookup_port (struct port_bucket *bucket,
>  {
>    struct port_info *pi;
>  
> -  pthread_mutex_lock (&_ports_lock);
>    pthread_rwlock_rdlock (&_ports_htable_lock);
>  
>    pi = hurd_ihash_find (&_ports_htable, port);
> @@ -38,10 +37,9 @@ ports_lookup_port (struct port_bucket *bucket,
>      pi = 0;
>  
>    if (pi)
> -    pi->refcnt++;
> +    refcounts_unsafe_ref (&pi->refcounts, NULL);
>  
>    pthread_rwlock_unlock (&_ports_htable_lock);
> -  pthread_mutex_unlock (&_ports_lock);
>  
>    return pi;
>  }
> diff --git a/libports/port-deref-weak.c b/libports/port-deref-weak.c
> index beb4842..8432660 100644
> --- a/libports/port-deref-weak.c
> +++ b/libports/port-deref-weak.c
> @@ -25,12 +25,8 @@ void
>  ports_port_deref_weak (void *portstruct)
>  {
>    struct port_info *pi = portstruct;
> -  
> -  pthread_mutex_lock (&_ports_lock);
> -  assert (pi->weakrefcnt);
> -  pi->weakrefcnt--;
> -  if (pi->refcnt == 0 && pi->weakrefcnt == 0)
> +  struct references result;
> +  refcounts_deref_weak (&pi->refcounts, &result);
> +  if (result.hard == 0 && result.weak == 0)
>      _ports_complete_deallocate (pi);
> -  else
> -    pthread_mutex_unlock (&_ports_lock);
>  }
> diff --git a/libports/port-deref.c b/libports/port-deref.c
> index cf9b238..b97dd13 100644
> --- a/libports/port-deref.c
> +++ b/libports/port-deref.c
> @@ -25,26 +25,24 @@ void
>  ports_port_deref (void *portstruct)
>  {
>    struct port_info *pi = portstruct;
> -  int trieddroppingweakrefs = 0;
> -
> - retry:
> -  
> -  pthread_mutex_lock (&_ports_lock);
> -  
> -  if (pi->refcnt == 1 && pi->weakrefcnt
> -      && pi->class->dropweak_routine && !trieddroppingweakrefs)
> +  struct references result;
> +
> +  if (pi->class->dropweak_routine)
>      {
> -      pthread_mutex_unlock (&_ports_lock);
> -      (*pi->class->dropweak_routine) (pi);
> -      trieddroppingweakrefs = 1;
> -      goto retry;
> +      /* If we need to call the dropweak routine, we need to hold one
> +         reference while doing so.  We use a weak reference for this
> +         purpose, which we acquire by demoting our hard reference to a
> +         weak one.  */
> +      refcounts_demote (&pi->refcounts, &result);
> +
> +      if (result.hard == 0 && result.weak > 1)
> +        (*pi->class->dropweak_routine) (pi);
> +
> +      refcounts_deref_weak (&pi->refcounts, &result);
>      }
> -  
> -  assert (pi->refcnt);
> +  else
> +    refcounts_deref (&pi->refcounts, &result);
>  
> -  pi->refcnt--;
> -  if (pi->refcnt == 0 && pi->weakrefcnt == 0)
> +  if (result.hard == 0 && result.weak == 0)
>      _ports_complete_deallocate (pi);
> -  else
> -    pthread_mutex_unlock (&_ports_lock);
>  }
> diff --git a/libports/port-ref-weak.c b/libports/port-ref-weak.c
> index c7d3c69..3f62dfe 100644
> --- a/libports/port-ref-weak.c
> +++ b/libports/port-ref-weak.c
> @@ -25,9 +25,5 @@ void
>  ports_port_ref_weak (void *portstruct)
>  {
>    struct port_info *pi = portstruct;
> -  
> -  pthread_mutex_lock (&_ports_lock);
> -  assert (pi->refcnt || pi->weakrefcnt);
> -  pi->weakrefcnt++;
> -  pthread_mutex_unlock (&_ports_lock);
> +  refcounts_ref_weak (&pi->refcounts, NULL);
>  }
> diff --git a/libports/port-ref.c b/libports/port-ref.c
> index 92b7118..9a1c71e 100644
> --- a/libports/port-ref.c
> +++ b/libports/port-ref.c
> @@ -25,9 +25,5 @@ void
>  ports_port_ref (void *portstruct)
>  {
>    struct port_info *pi = portstruct;
> -  
> -  pthread_mutex_lock (&_ports_lock);
> -  assert (pi->refcnt || pi->weakrefcnt);
> -  pi->refcnt++;
> -  pthread_mutex_unlock (&_ports_lock);
> +  refcounts_ref (&pi->refcounts, NULL);
>  }
> diff --git a/libports/ports.h b/libports/ports.h
> index 6922162..40d3b43 100644
> --- a/libports/ports.h
> +++ b/libports/ports.h
> @@ -27,6 +27,7 @@
>  #include <hurd/ihash.h>
>  #include <mach/notify.h>
>  #include <pthread.h>
> +#include <refcount.h>
>  
>  /* These are global values for common flags used in the various structures.
>     Not all of these are meaningful in all flag fields.  */
> @@ -39,8 +40,7 @@
>  struct port_info
>  {
>    struct port_class *class;
> -  int refcnt;
> -  int weakrefcnt;
> +  refcounts_t refcounts;
>    mach_port_mscount_t mscount;
>    mach_msg_seqno_t cancel_threshold;
>    int flags;
> diff --git a/libports/reallocate-from-external.c 
> b/libports/reallocate-from-external.c
> index 9944b39..7205bd9 100644
> --- a/libports/reallocate-from-external.c
> +++ b/libports/reallocate-from-external.c
> @@ -56,7 +56,7 @@ ports_reallocate_from_external (void *portstruct, 
> mach_port_t receive)
>    else if (((pi->flags & PORT_HAS_SENDRIGHTS) == 0) && stat.mps_srights)
>      {
>        pi->flags |= PORT_HAS_SENDRIGHTS;
> -      pi->refcnt++;
> +      refcounts_ref (&pi->refcounts, NULL);
>      }
>    
>    pi->port_right = receive;
> diff --git a/libports/transfer-right.c b/libports/transfer-right.c
> index 3f48290..776a8d2 100644
> --- a/libports/transfer-right.c
> +++ b/libports/transfer-right.c
> @@ -72,7 +72,7 @@ ports_transfer_right (void *tostruct,
>        else if (((topi->flags & PORT_HAS_SENDRIGHTS) == 0) && hassendrights)
>       {
>         topi->flags |= PORT_HAS_SENDRIGHTS;
> -       topi->refcnt++;
> +       refcounts_ref (&topi->refcounts, NULL);
>       }
>      }
>    
> diff --git a/utils/rpctrace.c b/utils/rpctrace.c
> index fc913e3..b11fea4 100644
> --- a/utils/rpctrace.c
> +++ b/utils/rpctrace.c
> @@ -431,7 +431,9 @@ destroy_receiver_info (struct receiver_info *info)
>    while (send_wrapper)
>      {
>        struct sender_info *next = send_wrapper->next;
> -      assert (TRACED_INFO (send_wrapper)->pi.refcnt == 1);
> +      assert (
> +     refcounts_hard_references (&TRACED_INFO (send_wrapper)->pi.refcounts)
> +     == 1);
>        /* Reset the receive_right of the send wrapper in advance to avoid
>         * destroy_receiver_info is called when the port info is destroyed. */
>        send_wrapper->receive_right = NULL;
> @@ -848,7 +850,11 @@ rewrite_right (mach_port_t *right, mach_msg_type_name_t 
> *type,
>           hurd_ihash_locp_remove (&traced_names, receiver_info->locp);
>  
>           send_wrapper2 = get_send_wrapper (receiver_info, dest, &rr);
> -         assert (TRACED_INFO (send_wrapper2)->pi.refcnt == 1);
> +         assert (
> +           refcounts_hard_references (
> +             &TRACED_INFO (send_wrapper2)->pi.refcounts)
> +           == 1);
> +
>           name = TRACED_INFO (send_wrapper2)->name;
>           TRACED_INFO (send_wrapper2)->name = NULL;
>           /* send_wrapper2 isn't destroyed normally, so we need to unlink
> -- 
> 2.1.0
> 

-- 
Samuel
I am the "ILOVEGNU" signature virus. Just copy me to your signature.
This email was infected under the terms of the GNU General Public License.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]