[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.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH 2/2] libports: lock-less reference counting for port_info objects,
Samuel Thibault <=