From a25b87dc26e266886f2da61f0fa96ede5dbd1fc5 Mon Sep 17 00:00:00 2001 From: Justus Winter <4winter@informatik.uni-hamburg.de> Date: Sat, 3 May 2014 01:02:35 +0200 Subject: [PATCH 9/9] libports: lock-less reference counting for port_info objects * 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 | 8 +++----- libports/port-ref.c | 8 +++----- libports/ports.h | 4 ++-- libports/reallocate-from-external.c | 2 +- libports/transfer-right.c | 2 +- utils/rpctrace.c | 10 ++++++++-- 14 files changed, 56 insertions(+), 52 deletions(-) diff --git a/libports/bucket-iterate.c b/libports/bucket-iterate.c index 88f082f..bf6857a 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) { 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..cff0546 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++; + ports_port_ref (pi); 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..e4b7fc8 100644 --- a/libports/port-ref-weak.c +++ b/libports/port-ref-weak.c @@ -25,9 +25,7 @@ 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); + struct references result; + refcounts_ref_weak (&pi->refcounts, &result); + assert (result.hard > 0 || result.weak > 1); } diff --git a/libports/port-ref.c b/libports/port-ref.c index 92b7118..761c50f 100644 --- a/libports/port-ref.c +++ b/libports/port-ref.c @@ -25,9 +25,7 @@ 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); + struct references result; + refcounts_ref (&pi->refcounts, &result); + assert (result.hard > 1 || result.weak > 0); } 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 #include #include +#include /* 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.0.0.rc2