From a4b491c53016ae4f0aeb3aff9321ec1274f0a61c Mon Sep 17 00:00:00 2001 From: Justus Winter <4winter@informatik.uni-hamburg.de> Date: Sat, 3 May 2014 03:53:41 +0200 Subject: [PATCH 7/8] libports: use a global hash table for the lookups Previously, libports used a hash table per port bucket. This makes looking up a port difficult if one does not know the port bucket, as one has to iterate over all buckets and do a hash table lookup each. Having to iterate over the buckets makes it necessary to keep a list of all buckets, which has to be updated and protected by a lock as well. Also, the current code in _ports_bucket_class_iterate iterates over the hash table associated with the bucket given. When ports_class_iterate calls this common function, it obtains a reference to the bucket from one of the ports in the given class. This will not work if a class contains ports in different port buckets. This limitation is not documented as far as I can see. Again, having to maintain this list has its cost and requires serialization. Use a global hash table for lookups instead. Keep the per-bucket hash tables for efficient iteration over buckets. Furthermore, serialize access to all hash tables using a separate lock. Remove the linked lists of all buckets and all ports in a class. * libports/bucket-iterate.c (ports_bucket_iterate): Acquire _ports_htable_lock. Also, generalize ports_bucket_iterate so that it takes a pointer to a hash table as first argument. (ports_bucket_iterate): Ajust call to former function accordingly. * libports/class-iterate.c (ports_class_iterate): Just call the generalized _ports_bucket_class_iterate with the global hash table as argument. * libports/ports.h (struct port_info): Remove the port class links. (struct port_bucket): Remove the hash table, and the all buckets link. (_ports_all_buckets): Remove declaration. (_ports_htable): New global hash table. (_ports_htable_lock): Protected by this lock. * libports/claim-right.c: Adjust accordingly. * libports/complete-deallocate.c: Likewise. * libports/create-bucket.c: Likewise. * libports/create-class.c: Likewise. * libports/create-internal.c: Likewise. * libports/destroy-right.c: Likewise. * libports/import-port.c: Likewise. * libports/lookup-port.c: Likewise. * libports/reallocate-from-external.c: Likewise. * libports/reallocate-port.c: Likewise. * libports/transfer-right.c: Likewise. * libports/inhibit-all-rpcs.c: Iterate over the hash table. * libports/inhibit-bucket-rpcs.c: Likewise, but filter using bucket. * libports/inhibit-class-rpcs.c: Likewise, but filter using class. * libports/init.c (_ports_htable): Initialize. (_ports_htable_lock): Likewise. --- libports/bucket-iterate.c | 16 +++++++++------- libports/claim-right.c | 5 ++++- libports/class-iterate.c | 10 +--------- libports/complete-deallocate.c | 7 +++---- libports/create-bucket.c | 6 ------ libports/create-class.c | 1 - libports/create-internal.c | 19 +++++++++++++------ libports/destroy-right.c | 5 +++-- libports/import-port.c | 19 +++++++++++++------ libports/inhibit-all-rpcs.c | 27 +++++++++++++-------------- libports/inhibit-bucket-rpcs.c | 3 ++- libports/inhibit-class-rpcs.c | 27 ++++++++++++++++++--------- libports/init.c | 7 ++++++- libports/lookup-port.c | 23 +++++++++-------------- libports/ports.h | 22 +++++++++++++++++----- libports/reallocate-from-external.c | 15 +++++++++++---- libports/reallocate-port.c | 9 ++++++++- libports/transfer-right.c | 18 ++++++++++++++---- 18 files changed, 144 insertions(+), 95 deletions(-) diff --git a/libports/bucket-iterate.c b/libports/bucket-iterate.c index 2d1b00d..79b6d72 100644 --- a/libports/bucket-iterate.c +++ b/libports/bucket-iterate.c @@ -25,7 +25,7 @@ /* Internal entrypoint for both ports_bucket_iterate and ports_class_iterate. If CLASS is non-null, call FUN only for ports in that class. */ error_t -_ports_bucket_class_iterate (struct port_bucket *bucket, +_ports_bucket_class_iterate (struct hurd_ihash *ht, struct port_class *class, error_t (*fun)(void *)) { @@ -36,23 +36,24 @@ _ports_bucket_class_iterate (struct port_bucket *bucket, error_t err; pthread_mutex_lock (&_ports_lock); + pthread_rwlock_rdlock (&_ports_htable_lock); - if (bucket->htable.nr_items == 0) + if (ht->nr_items == 0) { - pthread_mutex_unlock (&_ports_lock); + pthread_rwlock_unlock (&_ports_htable_lock); return 0; } - nr_items = bucket->htable.nr_items; + nr_items = ht->nr_items; p = malloc (nr_items * sizeof *p); if (p == NULL) { - pthread_mutex_unlock (&_ports_lock); + pthread_rwlock_unlock (&_ports_htable_lock); return ENOMEM; } n = 0; - HURD_IHASH_ITERATE (&bucket->htable, arg) + HURD_IHASH_ITERATE (ht, arg) { struct port_info *const pi = arg; @@ -63,6 +64,7 @@ _ports_bucket_class_iterate (struct port_bucket *bucket, n++; } } + pthread_rwlock_unlock (&_ports_htable_lock); pthread_mutex_unlock (&_ports_lock); if (n != 0 && n != nr_items) @@ -89,5 +91,5 @@ error_t ports_bucket_iterate (struct port_bucket *bucket, error_t (*fun)(void *)) { - return _ports_bucket_class_iterate (bucket, 0, fun); + return _ports_bucket_class_iterate (&bucket->htable, NULL, fun); } diff --git a/libports/claim-right.c b/libports/claim-right.c index 4851ea3..85592ff 100644 --- a/libports/claim-right.c +++ b/libports/claim-right.c @@ -34,10 +34,13 @@ ports_claim_right (void *portstruct) if (ret == MACH_PORT_NULL) return ret; - pthread_mutex_lock (&_ports_lock); + pthread_rwlock_wrlock (&_ports_htable_lock); + 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); err = mach_port_move_member (mach_task_self (), ret, MACH_PORT_NULL); assert_perror (err); + pthread_mutex_lock (&_ports_lock); pi->port_right = MACH_PORT_NULL; if (pi->flags & PORT_HAS_SENDRIGHTS) { diff --git a/libports/class-iterate.c b/libports/class-iterate.c index 1f8878a..df33818 100644 --- a/libports/class-iterate.c +++ b/libports/class-iterate.c @@ -23,13 +23,5 @@ error_t ports_class_iterate (struct port_class *class, error_t (*fun)(void *)) { - pthread_mutex_lock (&_ports_lock); - if (class->ports != 0) - { - struct port_bucket *bucket = class->ports->bucket; - pthread_mutex_unlock (&_ports_lock); - return _ports_bucket_class_iterate (bucket, class, fun); - } - pthread_mutex_unlock (&_ports_lock); - return 0; + return _ports_bucket_class_iterate (&_ports_htable, class, fun); } diff --git a/libports/complete-deallocate.c b/libports/complete-deallocate.c index 8ce095b..4768dab 100644 --- a/libports/complete-deallocate.c +++ b/libports/complete-deallocate.c @@ -29,16 +29,15 @@ _ports_complete_deallocate (struct port_info *pi) if (pi->port_right) { + pthread_rwlock_wrlock (&_ports_htable_lock); + 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; } - *pi->prevp = pi->next; - if (pi->next) - pi->next->prevp = pi->prevp; - pi->bucket->count--; pi->class->count--; diff --git a/libports/create-bucket.c b/libports/create-bucket.c index 52d50c3..2c5f1b6 100644 --- a/libports/create-bucket.c +++ b/libports/create-bucket.c @@ -48,11 +48,5 @@ ports_create_bucket () hurd_ihash_init (&ret->htable, offsetof (struct port_info, hentry)); ret->rpcs = ret->flags = ret->count = 0; - - pthread_mutex_lock (&_ports_lock); - ret->next = _ports_all_buckets; - _ports_all_buckets = ret; - pthread_mutex_unlock (&_ports_lock); - return ret; } diff --git a/libports/create-class.c b/libports/create-class.c index 12c8add..782f52b 100644 --- a/libports/create-class.c +++ b/libports/create-class.c @@ -39,7 +39,6 @@ ports_create_class (void (*clean_routine)(void *), cl->dropweak_routine = dropweak_routine; cl->flags = 0; cl->rpcs = 0; - cl->ports = NULL; cl->count = 0; cl->uninhibitable_rpcs = ports_default_uninhibitable_rpcs; diff --git a/libports/create-internal.c b/libports/create-internal.c index 8551297..8543986 100644 --- a/libports/create-internal.c +++ b/libports/create-internal.c @@ -81,15 +81,22 @@ _ports_create_port_internal (struct port_class *class, goto loop; } + pthread_rwlock_wrlock (&_ports_htable_lock); + err = hurd_ihash_add (&_ports_htable, port, pi); + if (err) + { + pthread_rwlock_unlock (&_ports_htable_lock); + goto lose; + } err = hurd_ihash_add (&bucket->htable, port, pi); if (err) - goto lose; + { + hurd_ihash_locp_remove (&_ports_htable, pi->ports_htable_entry); + pthread_rwlock_unlock (&_ports_htable_lock); + goto lose; + } + pthread_rwlock_unlock (&_ports_htable_lock); - pi->next = class->ports; - pi->prevp = &class->ports; - if (class->ports) - class->ports->prevp = &pi->next; - class->ports = pi; bucket->count++; class->count++; pthread_mutex_unlock (&_ports_lock); diff --git a/libports/destroy-right.c b/libports/destroy-right.c index 65e19c7..448b379 100644 --- a/libports/destroy-right.c +++ b/libports/destroy-right.c @@ -30,12 +30,13 @@ ports_destroy_right (void *portstruct) if (pi->port_right != MACH_PORT_NULL) { - pthread_mutex_lock (&_ports_lock); + pthread_rwlock_wrlock (&_ports_htable_lock); + 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); err = mach_port_mod_refs (mach_task_self (), pi->port_right, MACH_PORT_RIGHT_RECEIVE, -1); assert_perror (err); - pthread_mutex_unlock (&_ports_lock); pi->port_right = MACH_PORT_NULL; diff --git a/libports/import-port.c b/libports/import-port.c index 226f47e..2660672 100644 --- a/libports/import-port.c +++ b/libports/import-port.c @@ -75,15 +75,22 @@ ports_import_port (struct port_class *class, struct port_bucket *bucket, goto loop; } + pthread_rwlock_wrlock (&_ports_htable_lock); + err = hurd_ihash_add (&_ports_htable, port, pi); + if (err) + { + pthread_rwlock_unlock (&_ports_htable_lock); + goto lose; + } err = hurd_ihash_add (&bucket->htable, port, pi); if (err) - goto lose; + { + hurd_ihash_locp_remove (&_ports_htable, pi->ports_htable_entry); + pthread_rwlock_unlock (&_ports_htable_lock); + goto lose; + } + pthread_rwlock_unlock (&_ports_htable_lock); - pi->next = class->ports; - pi->prevp = &class->ports; - if (class->ports) - class->ports->prevp = &pi->next; - class->ports = pi; bucket->count++; class->count++; pthread_mutex_unlock (&_ports_lock); diff --git a/libports/inhibit-all-rpcs.c b/libports/inhibit-all-rpcs.c index d4a54ba..27e2ec5 100644 --- a/libports/inhibit-all-rpcs.c +++ b/libports/inhibit-all-rpcs.c @@ -36,24 +36,23 @@ ports_inhibit_all_rpcs () struct port_bucket *bucket; int this_one = 0; - for (bucket = _ports_all_buckets; bucket; bucket = bucket->next) + pthread_rwlock_rdlock (&_ports_htable_lock); + HURD_IHASH_ITERATE (&_ports_htable, portstruct) { - HURD_IHASH_ITERATE (&bucket->htable, portstruct) + struct rpc_info *rpc; + struct port_info *pi = portstruct; + + for (rpc = pi->current_rpcs; rpc; rpc = rpc->next) { - struct rpc_info *rpc; - struct port_info *pi = portstruct; - - for (rpc = pi->current_rpcs; rpc; rpc = rpc->next) - { - /* Avoid cancelling the calling thread if it's currently - handling a RPC. */ - if (rpc->thread == hurd_thread_self ()) - this_one = 1; - else - hurd_thread_cancel (rpc->thread); - } + /* Avoid cancelling the calling thread if it's currently + handling a RPC. */ + if (rpc->thread == hurd_thread_self ()) + this_one = 1; + else + hurd_thread_cancel (rpc->thread); } } + pthread_rwlock_unlock (&_ports_htable_lock); while (_ports_total_rpcs > this_one) { diff --git a/libports/inhibit-bucket-rpcs.c b/libports/inhibit-bucket-rpcs.c index 965aa03..82efdf5 100644 --- a/libports/inhibit-bucket-rpcs.c +++ b/libports/inhibit-bucket-rpcs.c @@ -35,6 +35,7 @@ ports_inhibit_bucket_rpcs (struct port_bucket *bucket) { int this_one = 0; + pthread_rwlock_rdlock (&_ports_htable_lock); HURD_IHASH_ITERATE (&bucket->htable, portstruct) { struct rpc_info *rpc; @@ -49,7 +50,7 @@ ports_inhibit_bucket_rpcs (struct port_bucket *bucket) hurd_thread_cancel (rpc->thread); } } - + pthread_rwlock_unlock (&_ports_htable_lock); while (bucket->rpcs > this_one) { diff --git a/libports/inhibit-class-rpcs.c b/libports/inhibit-class-rpcs.c index 7ee8653..9a87a5f 100644 --- a/libports/inhibit-class-rpcs.c +++ b/libports/inhibit-class-rpcs.c @@ -36,15 +36,24 @@ ports_inhibit_class_rpcs (struct port_class *class) struct rpc_info *rpc; int this_one = 0; - for (pi = class->ports; pi; pi = pi->next) - for (rpc = pi->current_rpcs; rpc; rpc = rpc->next) - { - /* Avoid cancelling the calling thread. */ - if (rpc->thread == hurd_thread_self ()) - this_one = 1; - else - hurd_thread_cancel (rpc->thread); - } + pthread_rwlock_rdlock (&_ports_htable_lock); + HURD_IHASH_ITERATE (&_ports_htable, portstruct) + { + struct rpc_info *rpc; + struct port_info *pi = portstruct; + if (pi->class != class) + continue; + + for (rpc = pi->current_rpcs; rpc; rpc = rpc->next) + { + /* Avoid cancelling the calling thread. */ + if (rpc->thread == hurd_thread_self ()) + this_one = 1; + else + hurd_thread_cancel (rpc->thread); + } + } + pthread_rwlock_unlock (&_ports_htable_lock); while (class->rpcs > this_one) { diff --git a/libports/init.c b/libports/init.c index 3ef5388..4a68cb8 100644 --- a/libports/init.c +++ b/libports/init.c @@ -19,9 +19,14 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ #include "ports.h" +#include pthread_mutex_t _ports_lock = PTHREAD_MUTEX_INITIALIZER; pthread_cond_t _ports_block = PTHREAD_COND_INITIALIZER; -struct port_bucket *_ports_all_buckets; + +struct hurd_ihash _ports_htable = + HURD_IHASH_INITIALIZER (offsetof (struct port_info, ports_htable_entry)); +pthread_rwlock_t _ports_htable_lock = PTHREAD_RWLOCK_INITIALIZER; + int _ports_total_rpcs; int _ports_flags; diff --git a/libports/lookup-port.c b/libports/lookup-port.c index f79f6f0..858ee11 100644 --- a/libports/lookup-port.c +++ b/libports/lookup-port.c @@ -26,27 +26,22 @@ ports_lookup_port (struct port_bucket *bucket, mach_port_t port, struct port_class *class) { - struct port_info *pi = 0; - + struct port_info *pi; + pthread_mutex_lock (&_ports_lock); + pthread_rwlock_rdlock (&_ports_htable_lock); - if (bucket) - pi = hurd_ihash_find (&bucket->htable, port); - else - for (bucket = _ports_all_buckets; bucket; bucket = bucket->next) - { - pi = hurd_ihash_find (&bucket->htable, port); - if (pi) - break; - } - - if (pi && class && pi->class != class) + pi = hurd_ihash_find (&_ports_htable, port); + if (pi + && ((class && pi->class != class) + || (bucket && pi->bucket != bucket))) pi = 0; if (pi) pi->refcnt++; + pthread_rwlock_unlock (&_ports_htable_lock); pthread_mutex_unlock (&_ports_lock); - + return pi; } diff --git a/libports/ports.h b/libports/ports.h index 7f13124..6922162 100644 --- a/libports/ports.h +++ b/libports/ports.h @@ -48,7 +48,7 @@ struct port_info struct rpc_info *current_rpcs; struct port_bucket *bucket; hurd_ihash_locp_t hentry; - struct port_info *next, **prevp; /* links on port_class list */ + hurd_ihash_locp_t ports_htable_entry; }; typedef struct port_info *port_info_t; @@ -61,11 +61,12 @@ typedef struct port_info *port_info_t; struct port_bucket { mach_port_t portset; + /* Per-bucket hash table used for fast iteration. Access must be + serialized using _ports_htable_lock. */ struct hurd_ihash htable; int rpcs; int flags; int count; - struct port_bucket *next; }; /* FLAGS above are the following: */ #define PORT_BUCKET_INHIBITED PORTS_INHIBITED @@ -78,7 +79,6 @@ struct port_class { int flags; int rpcs; - struct port_info *ports; int count; void (*clean_routine) (void *); void (*dropweak_routine) (void *); @@ -277,7 +277,7 @@ error_t ports_class_iterate (struct port_class *class, error_t (*fun)(void *port)); /* Internal entrypoint for above two. */ -error_t _ports_bucket_class_iterate (struct port_bucket *bucket, +error_t _ports_bucket_class_iterate (struct hurd_ihash *ht, struct port_class *class, error_t (*fun)(void *port)); @@ -402,7 +402,19 @@ extern kern_return_t /* Private data */ extern pthread_mutex_t _ports_lock; extern pthread_cond_t _ports_block; -extern struct port_bucket *_ports_all_buckets; + +/* A global hash table mapping port names to port_info objects. This + table is used for port lookups and to iterate over classes. + + A port in this hash table carries an implicit light reference. + When the reference counts reach zero, we call + _ports_complete_deallocate. There we reacquire our lock + momentarily to check whether someone else reacquired a reference + through the hash table. */ +extern struct hurd_ihash _ports_htable; +/* Access to the hash table is protected by this lock. */ +extern pthread_rwlock_t _ports_htable_lock; + extern int _ports_total_rpcs; extern int _ports_flags; #define _PORTS_INHIBITED PORTS_INHIBITED diff --git a/libports/reallocate-from-external.c b/libports/reallocate-from-external.c index 8cccb2a..9944b39 100644 --- a/libports/reallocate-from-external.c +++ b/libports/reallocate-from-external.c @@ -43,8 +43,11 @@ ports_reallocate_from_external (void *portstruct, mach_port_t receive) MACH_PORT_RIGHT_RECEIVE, -1); assert_perror (err); + pthread_rwlock_wrlock (&_ports_htable_lock); + 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); + if ((pi->flags & PORT_HAS_SENDRIGHTS) && !stat.mps_srights) { dropref = 1; @@ -59,11 +62,15 @@ ports_reallocate_from_external (void *portstruct, mach_port_t receive) pi->port_right = receive; pi->cancel_threshold = 0; pi->mscount = stat.mps_mscount; - - err = hurd_ihash_add (&pi->bucket->htable, receive, pi); + + pthread_rwlock_wrlock (&_ports_htable_lock); + err = hurd_ihash_add (&_ports_htable, receive, pi); assert_perror (err); + err = hurd_ihash_add (&pi->bucket->htable, receive, pi); + pthread_rwlock_unlock (&_ports_htable_lock); pthread_mutex_unlock (&_ports_lock); - + assert_perror (err); + mach_port_move_member (mach_task_self (), receive, pi->bucket->portset); if (stat.mps_srights) diff --git a/libports/reallocate-port.c b/libports/reallocate-port.c index d2adaeb..cc534eb 100644 --- a/libports/reallocate-port.c +++ b/libports/reallocate-port.c @@ -36,7 +36,10 @@ ports_reallocate_port (void *portstruct) MACH_PORT_RIGHT_RECEIVE, -1); assert_perror (err); + pthread_rwlock_wrlock (&_ports_htable_lock); + 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); err = mach_port_allocate (mach_task_self (), MACH_PORT_RIGHT_RECEIVE, &pi->port_right); @@ -48,9 +51,13 @@ ports_reallocate_port (void *portstruct) } pi->cancel_threshold = 0; pi->mscount = 0; - err = hurd_ihash_add (&pi->bucket->htable, pi->port_right, pi); + pthread_rwlock_wrlock (&_ports_htable_lock); + err = hurd_ihash_add (&_ports_htable, pi->port_right, pi); assert_perror (err); + err = hurd_ihash_add (&pi->bucket->htable, pi->port_right, pi); + pthread_rwlock_unlock (&_ports_htable_lock); pthread_mutex_unlock (&_ports_lock); + assert_perror (err); err = mach_port_move_member (mach_task_self (), pi->port_right, pi->bucket->portset); diff --git a/libports/transfer-right.c b/libports/transfer-right.c index 72488a9..3f48290 100644 --- a/libports/transfer-right.c +++ b/libports/transfer-right.c @@ -41,7 +41,10 @@ ports_transfer_right (void *tostruct, port = frompi->port_right; if (port != MACH_PORT_NULL) { + pthread_rwlock_wrlock (&_ports_htable_lock); + hurd_ihash_locp_remove (&_ports_htable, frompi->ports_htable_entry); hurd_ihash_locp_remove (&frompi->bucket->htable, frompi->hentry); + pthread_rwlock_unlock (&_ports_htable_lock); frompi->port_right = MACH_PORT_NULL; if (frompi->flags & PORT_HAS_SENDRIGHTS) { @@ -54,7 +57,10 @@ ports_transfer_right (void *tostruct, /* Destroy the existing right in TOPI. */ if (topi->port_right != MACH_PORT_NULL) { + pthread_rwlock_wrlock (&_ports_htable_lock); + hurd_ihash_locp_remove (&_ports_htable, topi->ports_htable_entry); hurd_ihash_locp_remove (&topi->bucket->htable, topi->hentry); + pthread_rwlock_unlock (&_ports_htable_lock); err = mach_port_mod_refs (mach_task_self (), topi->port_right, MACH_PORT_RIGHT_RECEIVE, -1); assert_perror (err); @@ -74,10 +80,16 @@ ports_transfer_right (void *tostruct, topi->port_right = port; topi->cancel_threshold = frompi->cancel_threshold; topi->mscount = frompi->mscount; - + + pthread_mutex_unlock (&_ports_lock); + if (port) { + pthread_rwlock_wrlock (&_ports_htable_lock); + err = hurd_ihash_add (&_ports_htable, port, topi); + assert_perror (err); err = hurd_ihash_add (&topi->bucket->htable, port, topi); + pthread_rwlock_unlock (&_ports_htable_lock); assert_perror (err); if (topi->bucket != frompi->bucket) { @@ -86,9 +98,7 @@ ports_transfer_right (void *tostruct, assert_perror (err); } } - - pthread_mutex_unlock (&_ports_lock); - + /* Take care of any lowered reference counts. */ if (dereffrompi) ports_port_deref (frompi); -- 2.0.0