[haiku-commits] r40235 - in haiku/branches/developer/bonefish/signals: headers/private/kernel src/system/kernel

  • From: ingo_weinhold@xxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 14 Jan 2011 02:02:28 +0100 (CET)

Author: bonefish
Date: 2011-01-14 02:02:28 +0100 (Fri, 14 Jan 2011)
New Revision: 40235
Changeset: http://dev.haiku-os.org/changeset/40235
Ticket: http://dev.haiku-os.org/ticket/5790

Modified:
   
haiku/branches/developer/bonefish/signals/headers/private/kernel/thread_types.h
   haiku/branches/developer/bonefish/signals/src/system/kernel/port.cpp
Log:
Ports:
* Replaced the ports array by a hash table. Ports are now allocated from the
  heap in create_port().
* Changed the locking a bit: sPortsLock needs to be locked whenever a port is
  created, deleted, or looked up by ID, as well as in find_port() and
  _user_get_next_port_info(). We save scanning the potentially sparse array
  in the latter two functions and the rather complicated ID computation in
  create_port(), though. More importantly, however, the change fixes the race
  condition between delete_owned_ports() and delete_port()/set_port_owner(),
  which was caused by the former trying to avoid a locking order inversion
  by moving all of the team's ports to a temporary list.
* The port ID allocation handles integer overflows, now. Should fix #5790,
  although other IDs still have the same issue, I believe.


Modified: 
haiku/branches/developer/bonefish/signals/headers/private/kernel/thread_types.h
===================================================================
--- 
haiku/branches/developer/bonefish/signals/headers/private/kernel/thread_types.h 
    2011-01-13 20:18:18 UTC (rev 40234)
+++ 
haiku/branches/developer/bonefish/signals/headers/private/kernel/thread_types.h 
    2011-01-14 01:02:28 UTC (rev 40235)
@@ -268,7 +268,7 @@
        struct list             image_list;             // protected by 
sImageMutex
        struct list             watcher_list;
        struct list             sem_list;
-       struct list             port_list;              // protected by fLock
+       struct list             port_list;              // protected by 
sPortsLock
        struct arch_team arch_info;
 
        addr_t                  user_data;

Modified: haiku/branches/developer/bonefish/signals/src/system/kernel/port.cpp
===================================================================
--- haiku/branches/developer/bonefish/signals/src/system/kernel/port.cpp        
2011-01-13 20:18:18 UTC (rev 40234)
+++ haiku/branches/developer/bonefish/signals/src/system/kernel/port.cpp        
2011-01-14 01:02:28 UTC (rev 40235)
@@ -1,4 +1,5 @@
 /*
+ * Copyright 2011, Ingo Weinhold, ingo_weinhold@xxxxxxx
  * Copyright 2002-2010, Axel Dörfler, axeld@xxxxxxxxxxxxxxxxx
  * Distributed under the terms of the MIT License.
  *
@@ -19,6 +20,8 @@
 
 #include <OS.h>
 
+#include <AutoDeleter.h>
+
 #include <arch/int.h>
 #include <heap.h>
 #include <kernel.h>
@@ -41,6 +44,23 @@
 #endif
 
 
+// Locking:
+// * sPortsLock: Protects the sPorts hash table, Team::port_list, and
+//   Port::owner.
+// * Port::lock: Protects all Port members save team_link, hash_link, and lock.
+//   id is immutable.
+//
+// The locking order is sPortsLock -> Port::lock. A port must be looked up
+// in sPorts and locked with sPortsLock held. Afterwards sPortsLock can be
+// dropped, unless any field guarded by sPortsLock is accessed.
+
+
+struct port_message;
+
+
+static void put_port_message(port_message* message);
+
+
 struct port_message : DoublyLinkedListLinkImpl<port_message> {
        int32                           code;
        size_t                          size;
@@ -52,8 +72,10 @@
 
 typedef DoublyLinkedList<port_message> MessageList;
 
-struct port_entry {
+
+struct Port {
        struct list_link        team_link;
+       Port*                           hash_link;
        port_id                         id;
        team_id                         owner;
        int32                           capacity;
@@ -66,8 +88,62 @@
                // messages read from port since creation
        select_info*            select_infos;
        MessageList                     messages;
+
+       Port(team_id owner, int32 queueLength, char* name)
+               :
+               owner(owner),
+               capacity(queueLength),
+               read_count(0),
+               write_count(queueLength),
+               total_count(0),
+               select_infos(NULL)
+       {
+               // id is initialized when the caller adds the port to the hash 
table
+
+               mutex_init(&lock, name);
+               read_condition.Init(this, "port read");
+               write_condition.Init(this, "port write");
+       }
+
+       ~Port()
+       {
+               while (port_message* message = messages.RemoveHead())
+                       put_port_message(message);
+
+               free((char*)lock.name);
+               lock.name = NULL;
+       }
 };
 
+
+struct PortHashDefinition {
+       typedef port_id         KeyType;
+       typedef Port            ValueType;
+
+       size_t HashKey(port_id key) const
+       {
+               return key;
+       }
+
+       size_t Hash(Port* value) const
+       {
+               return HashKey(value->id);
+       }
+
+       bool Compare(port_id key, Port* value) const
+       {
+               return value->id == key;
+       }
+
+       Port*& GetLink(Port* value) const
+       {
+               return value->hash_link;
+       }
+};
+
+typedef BOpenHashTable<PortHashDefinition> PortHashTable;
+
+
 class PortNotificationService : public DefaultNotificationService {
 public:
                                                        
PortNotificationService();
@@ -81,13 +157,13 @@
 
 class Create : public AbstractTraceEntry {
 public:
-       Create(port_entry& port)
+       Create(Port* port)
                :
-               fID(port.id),
-               fOwner(port.owner),
-               fCapacity(port.capacity)
+               fID(port->id),
+               fOwner(port->owner),
+               fCapacity(port->capacity)
        {
-               fName = alloc_tracing_buffer_strcpy(port.lock.name, 
B_OS_NAME_LENGTH,
+               fName = alloc_tracing_buffer_strcpy(port->lock.name, 
B_OS_NAME_LENGTH,
                        false);
 
                Initialized();
@@ -109,9 +185,9 @@
 
 class Delete : public AbstractTraceEntry {
 public:
-       Delete(port_entry& port)
+       Delete(Port* port)
                :
-               fID(port.id)
+               fID(port->id)
        {
                Initialized();
        }
@@ -128,11 +204,12 @@
 
 class Read : public AbstractTraceEntry {
 public:
-       Read(port_entry& port, int32 code, ssize_t result)
+       Read(port_id id, int32 readCount, int32 writeCount, int32 code,
+               ssize_t result)
                :
-               fID(port.id),
-               fReadCount(port.read_count),
-               fWriteCount(port.write_count),
+               fID(id),
+               fReadCount(readCount),
+               fWriteCount(writeCount),
                fCode(code),
                fResult(result)
        {
@@ -156,11 +233,12 @@
 
 class Write : public AbstractTraceEntry {
 public:
-       Write(port_entry& port, int32 code, size_t bufferSize, ssize_t result)
+       Write(port_id id, int32 readCount, int32 writeCount, int32 code,
+               size_t bufferSize, ssize_t result)
                :
-               fID(port.id),
-               fReadCount(port.read_count),
-               fWriteCount(port.write_count),
+               fID(id),
+               fReadCount(readCount),
+               fWriteCount(writeCount),
                fCode(code),
                fBufferSize(bufferSize),
                fResult(result)
@@ -186,11 +264,12 @@
 
 class Info : public AbstractTraceEntry {
 public:
-       Info(port_entry& port, int32 code, ssize_t result)
+       Info(port_id id, int32 readCount, int32 writeCount, int32 code,
+               ssize_t result)
                :
-               fID(port.id),
-               fReadCount(port.read_count),
-               fWriteCount(port.write_count),
+               fID(id),
+               fReadCount(readCount),
+               fWriteCount(writeCount),
                fCode(code),
                fResult(result)
        {
@@ -214,10 +293,10 @@
 
 class OwnerChange : public AbstractTraceEntry {
 public:
-       OwnerChange(port_entry& port, team_id newOwner, status_t status)
+       OwnerChange(Port* port, team_id newOwner, status_t status)
                :
-               fID(port.id),
-               fOldOwner(port.owner),
+               fID(port->id),
+               fOldOwner(port->owner),
                fNewOwner(newOwner),
                fStatus(status)
        {
@@ -253,20 +332,17 @@
 #define MAX_QUEUE_LENGTH 4096
 #define PORT_MAX_MESSAGE_SIZE (256 * 1024)
 
-// sMaxPorts must be power of 2
 static int32 sMaxPorts = 4096;
 static int32 sUsedPorts = 0;
 
-static struct port_entry* sPorts;
-static area_id sPortArea;
+static PortHashTable sPorts;
 static heap_allocator* sPortAllocator;
 static ConditionVariable sNoSpaceCondition;
 static vint32 sTotalSpaceInUse;
 static vint32 sAreaChangeCounter;
 static vint32 sAllocatingArea;
+static port_id sNextPortID = 1;
 static bool sPortsActive = false;
-static port_id sNextPort = 1;
-static int32 sFirstFreeSlot = 1;
 static mutex sPortsLock = MUTEX_INITIALIZER("ports list");
 
 static PortNotificationService sNotificationService;
@@ -303,7 +379,6 @@
 {
        const char* name = NULL;
        team_id owner = -1;
-       int32 i;
 
        if (argc > 2) {
                if (!strcmp(argv[1], "team") || !strcmp(argv[1], "owner"))
@@ -316,10 +391,9 @@
        kprintf("port             id  cap  read-cnt  write-cnt   total   team  "
                "name\n");
 
-       for (i = 0; i < sMaxPorts; i++) {
-               struct port_entry* port = &sPorts[i];
-               if (port->id < 0
-                       || (owner != -1 && port->owner != owner)
+       for (PortHashTable::Iterator it = sPorts.GetIterator();
+               Port* port = it.Next();) {
+               if ((owner != -1 && port->owner != owner)
                        || (name != NULL && strstr(port->lock.name, name) == 
NULL))
                        continue;
 
@@ -333,7 +407,7 @@
 
 
 static void
-_dump_port_info(struct port_entry* port)
+_dump_port_info(Port* port)
 {
        kprintf("PORT: %p\n", port);
        kprintf(" id:              %ld\n", port->id);
@@ -372,7 +446,7 @@
 
        if (argc > 2) {
                if (!strcmp(argv[1], "address")) {
-                       _dump_port_info((struct 
port_entry*)parse_expression(argv[2]));
+                       _dump_port_info((Port*)parse_expression(argv[2]));
                        return 0;
                } else if (!strcmp(argv[1], "condition"))
                        condition = 
(ConditionVariable*)parse_expression(argv[2]);
@@ -381,23 +455,24 @@
        } else if (parse_expression(argv[1]) > 0) {
                // if the argument looks like a number, treat it as such
                int32 num = parse_expression(argv[1]);
-               int32 slot = num % sMaxPorts;
-               if (sPorts[slot].id != num) {
+               Port* port = sPorts.Lookup(num);
+               if (port == NULL) {
                        kprintf("port %ld (%#lx) doesn't exist!\n", num, num);
                        return 0;
                }
-               _dump_port_info(&sPorts[slot]);
+               _dump_port_info(port);
                return 0;
        } else
                name = argv[1];
 
        // walk through the ports list, trying to match name
-       for (int32 i = 0; i < sMaxPorts; i++) {
-               if ((name != NULL && sPorts[i].lock.name != NULL
-                               && !strcmp(name, sPorts[i].lock.name))
-                       || (condition != NULL && (&sPorts[i].read_condition == 
condition
-                               || &sPorts[i].write_condition == condition))) {
-                       _dump_port_info(&sPorts[i]);
+       for (PortHashTable::Iterator it = sPorts.GetIterator();
+               Port* port = it.Next();) {
+               if ((name != NULL && port->lock.name != NULL
+                               && !strcmp(name, port->lock.name))
+                       || (condition != NULL && (&port->read_condition == 
condition
+                               || &port->write_condition == condition))) {
+                       _dump_port_info(port);
                        return 0;
                }
        }
@@ -406,11 +481,14 @@
 }
 
 
+/*!    Notifies the port's select events.
+       The port must be locked.
+*/
 static void
-notify_port_select_events(int slot, uint16 events)
+notify_port_select_events(Port* port, uint16 events)
 {
-       if (sPorts[slot].select_infos)
-               notify_select_events_list(sPorts[slot].select_infos, events);
+       if (port->select_infos)
+               notify_select_events_list(port->select_infos, events);
 }
 
 
@@ -512,19 +590,19 @@
 
 
 /*!    You need to own the port's lock when calling this function */
-static bool
-is_port_closed(int32 slot)
+static inline bool
+is_port_closed(Port* port)
 {
-       return sPorts[slot].capacity == 0;
+       return port->capacity == 0;
 }
 
 
 /*!    Fills the port_info structure with information from the specified
        port.
-       The port lock must be held when called.
+       The port's lock must be held when called.
 */
 static void
-fill_port_info(struct port_entry* port, port_info* info, size_t size)
+fill_port_info(Port* port, port_info* info, size_t size)
 {
        info->port = port->id;
        info->team = port->owner;
@@ -562,66 +640,64 @@
 
 
 static void
-uninit_port_locked(struct port_entry& port)
+uninit_port_locked(Port* port)
 {
-       int32 id = port.id;
+       notify_port_select_events(port, B_EVENT_INVALID);
+       port->select_infos = NULL;
 
-       // mark port as invalid
-       port.id = -1;
-       free((char*)port.lock.name);
-       port.lock.name = NULL;
+       // Release the threads that were blocking on this port.
+       // read_port() will see the B_BAD_PORT_ID return value, and act 
accordingly
+       port->read_condition.NotifyAll(false, B_BAD_PORT_ID);
+       port->write_condition.NotifyAll(false, B_BAD_PORT_ID);
+       sNotificationService.Notify(PORT_REMOVED, port->id);
+}
 
-       while (port_message* message = port.messages.RemoveHead()) {
-               put_port_message(message);
-       }
 
-       notify_port_select_events(id % sMaxPorts, B_EVENT_INVALID);
-       port.select_infos = NULL;
+static Port*
+get_locked_port(port_id id)
+{
+       MutexLocker portsLocker(sPortsLock);
 
-       // Release the threads that were blocking on this port.
-       // read_port() will see the B_BAD_PORT_ID return value, and act 
accordingly
-       port.read_condition.NotifyAll(false, B_BAD_PORT_ID);
-       port.write_condition.NotifyAll(false, B_BAD_PORT_ID);
-       sNotificationService.Notify(PORT_REMOVED, id);
+       Port* port = sPorts.Lookup(id);
+       if (port != NULL)
+               mutex_lock(&port->lock);
+       return port;
 }
 
 
 //     #pragma mark - private kernel API
 
 
-/*! This function delets all the ports that are owned by the passed team.
+/*! This function deletes all the ports that are owned by the passed team.
 */
 void
 delete_owned_ports(Team* team)
 {
        TRACE(("delete_owned_ports(owner = %ld)\n", team->id));
 
+       MutexLocker portsLocker(sPortsLock);
+
+       // move the ports from the team's port list to a local list
        struct list queue;
+       list_move_to_list(&team->port_list, &queue);
 
-       {
-               TeamLocker teamLocker(team);
-               list_move_to_list(&team->port_list, &queue);
-       }
+       // iterate through the list or ports, remove them from the hash table 
and
+       // uninitialize them
+       Port* port = (Port*)list_get_first_item(&queue);
+       while (port != NULL) {
+               MutexLocker locker(port->lock);
+               sPorts.Remove(port);
+               uninit_port_locked(port);
+               sUsedPorts--;
 
-       int32 firstSlot = sMaxPorts;
-       int32 count = 0;
-
-       while (port_entry* port = (port_entry*)list_remove_head_item(&queue)) {
-               if (firstSlot > port->id % sMaxPorts)
-                       firstSlot = port->id % sMaxPorts;
-               count++;
-
-               MutexLocker locker(port->lock);
-               uninit_port_locked(*port);
+               port = (Port*)list_get_next_item(&queue, port);
        }
 
-       MutexLocker _(sPortsLock);
+       portsLocker.Unlock();
 
-       // update the first free slot hint in the array
-       if (firstSlot < sFirstFreeSlot)
-               sFirstFreeSlot = firstSlot;
-
-       sUsedPorts -= count;
+       // delete the ports
+       while (Port* port = (Port*)list_remove_head_item(&queue))
+               delete port;
 }
 
 
@@ -642,28 +718,13 @@
 status_t
 port_init(kernel_args *args)
 {
-       size_t size = sizeof(struct port_entry) * sMaxPorts;
-
-       // create and initialize ports table
-       virtual_address_restrictions virtualRestrictions = {};
-       virtualRestrictions.address_specification = B_ANY_KERNEL_ADDRESS;
-       physical_address_restrictions physicalRestrictions = {};
-       sPortArea = create_area_etc(B_SYSTEM_TEAM, "port_table", size, 
B_FULL_LOCK,
-               B_KERNEL_READ_AREA | B_KERNEL_WRITE_AREA, CREATE_AREA_DONT_WAIT,
-               &virtualRestrictions, &physicalRestrictions, (void**)&sPorts);
-       if (sPortArea < 0) {
-               panic("unable to allocate kernel port table!\n");
-               return sPortArea;
+       // initialize ports table
+       new(&sPorts) PortHashTable;
+       if (sPorts.Init() != B_OK) {
+               panic("Failed to init port hash table!");
+               return B_NO_MEMORY;
        }
 
-       memset(sPorts, 0, size);
-       for (int32 i = 0; i < sMaxPorts; i++) {
-               mutex_init(&sPorts[i].lock, NULL);
-               sPorts[i].id = -1;
-               sPorts[i].read_condition.Init(&sPorts[i], "port read");
-               sPorts[i].write_condition.Init(&sPorts[i], "port write");
-       }
-
        addr_t base;
        if (create_area("port heap", (void**)&base, B_ANY_KERNEL_ADDRESS,
                        kInitialPortBufferSize, B_NO_LOCK,
@@ -686,7 +747,7 @@
                return B_NO_MEMORY;
        }
 
-       sNoSpaceCondition.Init(sPorts, "port space");
+       sNoSpaceCondition.Init(&sPorts, "port space");
 
        // add debugger commands
        add_debugger_command_etc("ports", &dump_port_list,
@@ -731,64 +792,53 @@
        if (team == NULL)
                return B_BAD_TEAM_ID;
 
-       MutexLocker locker(sPortsLock);
-
-       // check early on if there are any free port slots to use
-       if (sUsedPorts >= sMaxPorts)
-               return B_NO_MORE_PORTS;
-
        // check & dup name
        char* nameBuffer = strdup(name != NULL ? name : "unnamed port");
        if (nameBuffer == NULL)
                return B_NO_MEMORY;
 
-       sUsedPorts++;
+       // create a port
+       Port* port = new(std::nothrow) Port(team_get_current_team_id(), 
queueLength,
+               nameBuffer);
+       if (port == NULL) {
+               free(nameBuffer);
+               return B_NO_MEMORY;
+       }
+       ObjectDeleter<Port> portDeleter(port);
 
-       // find the first empty spot
-       for (int32 slot = 0; slot < sMaxPorts; slot++) {
-               int32 i = (slot + sFirstFreeSlot) % sMaxPorts;
+       MutexLocker locker(sPortsLock);
 
-               if (sPorts[i].id == -1) {
-                       // make the port_id be a multiple of the slot it's in
-                       if (i >= sNextPort % sMaxPorts)
-                               sNextPort += i - sNextPort % sMaxPorts;
-                       else
-                               sNextPort += sMaxPorts - (sNextPort % sMaxPorts 
- i);
-                       sFirstFreeSlot = slot + 1;
+       // check the ports limit
+       if (sUsedPorts >= sMaxPorts)
+               return B_NO_MORE_PORTS;
 
-                       MutexLocker portLocker(sPorts[i].lock);
-                       sPorts[i].id = sNextPort++;
-                       locker.Unlock();
+       sUsedPorts++;
 
-                       sPorts[i].capacity = queueLength;
-                       sPorts[i].owner = team_get_current_team_id();
-                       sPorts[i].lock.name = nameBuffer;
-                       sPorts[i].read_count = 0;
-                       sPorts[i].write_count = queueLength;
-                       sPorts[i].total_count = 0;
-                       sPorts[i].select_infos = NULL;
+       // allocate a port ID
+       do {
+               port->id = sNextPortID++;
 
-                       {
-                               TeamLocker teamLocker(team);
-                               list_add_item(&team->port_list, 
&sPorts[i].team_link);
-                       }
+               // handle integer overflow
+               if (sNextPortID < 0)
+                       sNextPortID = 1;
+       } while (sPorts.Lookup(port->id) != NULL);
 
-                       port_id id = sPorts[i].id;
+       // insert port in table and team list
+       sPorts.Insert(port);
+       list_add_item(&team->port_list, &port->team_link);
+       portDeleter.Detach();
 
-                       T(Create(sPorts[i]));
-                       portLocker.Unlock();
+       // tracing, notifications, etc.
+       T(Create(port));
 
-                       TRACE(("create_port() done: port created %ld\n", id));
+       port_id id = port->id;
 
-                       sNotificationService.Notify(PORT_ADDED, id);
-                       return id;
-               }
-       }
+       locker.Unlock();
 
-       // Still not enough ports... - due to sUsedPorts, this cannot really
-       // happen anymore.
-       panic("out of ports, but sUsedPorts is broken");
-       return B_NO_MORE_PORTS;
+       TRACE(("create_port() done: port created %ld\n", id));
+
+       sNotificationService.Notify(PORT_ADDED, id);
+       return id;
 }
 
 
@@ -800,25 +850,23 @@
        if (!sPortsActive || id < 0)
                return B_BAD_PORT_ID;
 
-       int32 slot = id % sMaxPorts;
-
-       // walk through the sem list, trying to match name
-       MutexLocker locker(sPorts[slot].lock);
-
-       if (sPorts[slot].id != id) {
+       // get the port
+       Port* port = get_locked_port(id);
+       if (port == NULL) {
                TRACE(("close_port: invalid port_id %ld\n", id));
                return B_BAD_PORT_ID;
        }
+       MutexLocker lock(&port->lock, true);
 
        // mark port to disable writing - deleting the semaphores will
        // wake up waiting read/writes
-       sPorts[slot].capacity = 0;
+       port->capacity = 0;
 
-       notify_port_select_events(slot, B_EVENT_INVALID);
-       sPorts[slot].select_infos = NULL;
+       notify_port_select_events(port, B_EVENT_INVALID);
+       port->select_infos = NULL;
 
-       sPorts[slot].read_condition.NotifyAll(false, B_BAD_PORT_ID);
-       sPorts[slot].write_condition.NotifyAll(false, B_BAD_PORT_ID);
+       port->read_condition.NotifyAll(false, B_BAD_PORT_ID);
+       port->write_condition.NotifyAll(false, B_BAD_PORT_ID);
 
        return B_OK;
 }
@@ -832,39 +880,34 @@
        if (!sPortsActive || id < 0)
                return B_BAD_PORT_ID;
 
-       int32 slot = id % sMaxPorts;
+       // get the port and remove it from the hash table and the team
+       Port* port;
+       MutexLocker locker;
+       {
+               MutexLocker portsLocker(sPortsLock);
 
-       MutexLocker locker(sPorts[slot].lock);
+               port = sPorts.Lookup(id);
+               if (port == NULL) {
+                       TRACE(("delete_port: invalid port_id %ld\n", id));
+                       return B_BAD_PORT_ID;
+               }
 
-       if (sPorts[slot].id != id) {
-               TRACE(("delete_port: invalid port_id %ld\n", id));
-               return B_BAD_PORT_ID;
-       }
+               sPorts.Remove(port);
+               list_remove_link(&port->team_link);
 
-       T(Delete(sPorts[slot]));
+               sUsedPorts--;
 
-       {
-               // get the owning team and remove the port from its ports list
-               Team* team = Team::Get(sPorts[slot].owner);
-               if (team == NULL)
-                       return B_BAD_PORT_ID;
-               BReference<Team> teamReference(team, true);
+               locker.SetTo(port->lock, false);
 
-               TeamLocker teamLocker(team);
-               list_remove_link(&sPorts[slot].team_link);
+               uninit_port_locked(port);
        }
 
-       uninit_port_locked(sPorts[slot]);
+       T(Delete(port));
 
        locker.Unlock();
 
-       MutexLocker _(sPortsLock);
+       delete port;
 
-       // update the first free slot hint in the array
-       if (slot < sFirstFreeSlot)
-               sFirstFreeSlot = slot;
-
-       sUsedPorts--;
        return B_OK;
 }
 
@@ -875,13 +918,17 @@
        if (id < 0)
                return B_BAD_PORT_ID;
 
-       int32 slot = id % sMaxPorts;
+       // get the port
+       Port* port = get_locked_port(id);
+       if (port == NULL)
+               return B_BAD_PORT_ID;
+       MutexLocker locker(port->lock, true);
 
-       MutexLocker locker(sPorts[slot].lock);
+       // port must not yet be closed
+       if (is_port_closed(port))
+               return B_BAD_PORT_ID;
 
-       if (sPorts[slot].id != id || is_port_closed(slot))
-               return B_BAD_PORT_ID;
-       if (!kernel && sPorts[slot].owner == team_get_kernel_team_id()) {
+       if (!kernel && port->owner == team_get_kernel_team_id()) {
                // kernel port, but call from userland
                return B_NOT_ALLOWED;
        }
@@ -891,16 +938,16 @@
        if (info->selected_events != 0) {
                uint16 events = 0;
 
-               info->next = sPorts[slot].select_infos;
-               sPorts[slot].select_infos = info;
+               info->next = port->select_infos;
+               port->select_infos = info;
 
                // check for events
                if ((info->selected_events & B_EVENT_READ) != 0
-                       && !sPorts[slot].messages.IsEmpty()) {
+                       && !port->messages.IsEmpty()) {
                        events |= B_EVENT_READ;
                }
 
-               if (sPorts[slot].write_count > 0)
+               if (port->write_count > 0)
                        events |= B_EVENT_WRITE;
 
                if (events != 0)
@@ -919,19 +966,20 @@
        if (info->selected_events == 0)
                return B_OK;
 
-       int32 slot = id % sMaxPorts;
+       // get the port
+       Port* port = get_locked_port(id);
+       if (port == NULL)
+               return B_BAD_PORT_ID;
+       MutexLocker locker(port->lock, true);
 
-       MutexLocker locker(sPorts[slot].lock);
+       // find and remove the infos
+       select_info** infoLocation = &port->select_infos;
+       while (*infoLocation != NULL && *infoLocation != info)
+               infoLocation = &(*infoLocation)->next;
 
-       if (sPorts[slot].id == id) {
-               select_info** infoLocation = &sPorts[slot].select_infos;
-               while (*infoLocation != NULL && *infoLocation != info)
-                       infoLocation = &(*infoLocation)->next;
+       if (*infoLocation == info)
+               *infoLocation = info->next;
 
-               if (*infoLocation == info)
-                       *infoLocation = info->next;
-       }
-
        return B_OK;
 }
 
@@ -948,17 +996,12 @@
        if (name == NULL)
                return B_BAD_VALUE;
 
-       // Since we have to check every single port, and we don't
-       // care if it goes away at any point, we're only grabbing
-       // the port lock in question, not the port list lock
+       MutexLocker portsLocker(sPortsLock);
 
-       // loop over list
-       for (int32 i = 0; i < sMaxPorts; i++) {
-               // lock every individual port before comparing
-               MutexLocker _(sPorts[i].lock);
-
-               if (sPorts[i].id >= 0 && !strcmp(name, sPorts[i].lock.name))
-                       return sPorts[i].id;
+       for (PortHashTable::Iterator it = sPorts.GetIterator();
+               Port* port = it.Next();) {
+               if (!strcmp(name, port->lock.name))
+                       return port->id;
        }
 
        return B_NAME_NOT_FOUND;
@@ -975,60 +1018,64 @@
        if (!sPortsActive || id < 0)
                return B_BAD_PORT_ID;
 
-       int32 slot = id % sMaxPorts;
-
-       MutexLocker locker(sPorts[slot].lock);
-
-       if (sPorts[slot].id != id || sPorts[slot].capacity == 0) {
+       // get the port
+       Port* port = get_locked_port(id);
+       if (port == NULL) {
                TRACE(("get_port_info: invalid port_id %ld\n", id));
                return B_BAD_PORT_ID;
        }
+       MutexLocker locker(port->lock, true);
 
        // fill a port_info struct with info
-       fill_port_info(&sPorts[slot], info, size);
+       fill_port_info(port, info, size);
        return B_OK;
 }
 
 
 status_t
-_get_next_port_info(team_id team, int32* _cookie, struct port_info* info,
+_get_next_port_info(team_id teamID, int32* _cookie, struct port_info* info,
        size_t size)
 {
-       TRACE(("get_next_port_info(team = %ld)\n", team));
+       TRACE(("get_next_port_info(team = %ld)\n", teamID));
 
        if (info == NULL || size != sizeof(port_info) || _cookie == NULL
-               || team < B_OK)
+               || teamID < 0) {
                return B_BAD_VALUE;
+       }
        if (!sPortsActive)
                return B_BAD_PORT_ID;
 
-       int32 slot = *_cookie;
-       if (slot >= sMaxPorts)
-               return B_BAD_PORT_ID;
+       Team* team = Team::Get(teamID);
+       if (team == NULL)
+               return B_BAD_TEAM_ID;
+       BReference<Team> teamReference(team, true);
 
-       if (team == B_CURRENT_TEAM)
-               team = team_get_current_team_id();
+       // iterate through the team's port list
+       MutexLocker portsLocker(sPortsLock);
 
-       info->port = -1; // used as found flag
+       int32 stopIndex = *_cookie;
+       int32 index = 0;
 
-       while (slot < sMaxPorts) {
-               MutexLocker locker(sPorts[slot].lock);
-
-               if (sPorts[slot].id != -1 && !is_port_closed(slot)
-                       && sPorts[slot].owner == team) {
-                       // found one!
-                       fill_port_info(&sPorts[slot], info, size);
-                       slot++;
-                       break;
+       Port* port = (Port*)list_get_first_item(&team->port_list);
+       while (port != NULL) {
+               if (!is_port_closed(port)) {
+                       if (index == stopIndex)
+                               break;
+                       index++;
                }
 
-               slot++;
+               port = (Port*)list_get_next_item(&team->port_list, port);
        }
 
-       if (info->port == -1)
+       if (port == NULL)
                return B_BAD_PORT_ID;
 
-       *_cookie = slot;
+       // fill in the port info
+       MutexLocker locker(port->lock);
+       portsLocker.Unlock();
+       fill_port_info(port, info, size);
+
+       *_cookie = stopIndex + 1;
        return B_OK;
 }
 
@@ -1060,25 +1107,26 @@
 
        flags &= B_CAN_INTERRUPT | B_KILL_CAN_INTERRUPT | B_RELATIVE_TIMEOUT
                | B_ABSOLUTE_TIMEOUT;
-       int32 slot = id % sMaxPorts;
 
-       MutexLocker locker(sPorts[slot].lock);
+       // get the port
+       Port* port = get_locked_port(id);
+       if (port == NULL)
+               return B_BAD_PORT_ID;
+       MutexLocker locker(port->lock, true);
 
-       if (sPorts[slot].id != id
-               || (is_port_closed(slot) && sPorts[slot].messages.IsEmpty())) {
-               T(Info(sPorts[slot], 0, B_BAD_PORT_ID));
-               TRACE(("_get_port_message_info_etc(): %s port %ld\n",
-                       sPorts[slot].id == id ? "closed" : "invalid", id));
+       if (is_port_closed(port) && port->messages.IsEmpty()) {
+               T(Info(port, 0, B_BAD_PORT_ID));
+               TRACE(("_get_port_message_info_etc(): closed port %ld\n", id));
                return B_BAD_PORT_ID;
        }
 
-       while (sPorts[slot].read_count == 0) {
+       while (port->read_count == 0) {
                // We need to wait for a message to appear
                if ((flags & B_RELATIVE_TIMEOUT) != 0 && timeout <= 0)
                        return B_WOULD_BLOCK;
 
                ConditionVariableEntry entry;
-               sPorts[slot].read_condition.Add(&entry);
+               port->read_condition.Add(&entry);
 
                locker.Unlock();
 
@@ -1086,24 +1134,30 @@
                status_t status = entry.Wait(flags, timeout);
 
                if (status != B_OK) {
-                       T(Info(sPorts[slot], 0, status));
+                       T(Info(port, 0, status));
                        return status;
                }
 
-               locker.Lock();
+               // re-lock
+               Port* newPort = get_locked_port(id);
+               if (newPort == NULL) {
+                       T(Info(id, 0, 0, 0, B_BAD_PORT_ID));
+                       return B_BAD_PORT_ID;
+               }
+               locker.SetTo(newPort->lock, true);
 
-               if (sPorts[slot].id != id
-                       || (is_port_closed(slot) && 
sPorts[slot].messages.IsEmpty())) {
+               if (newPort != port
+                       || (is_port_closed(port) && port->messages.IsEmpty())) {
                        // the port is no longer there
-                       T(Info(sPorts[slot], 0, B_BAD_PORT_ID));
+                       T(Info(id, 0, 0, 0, B_BAD_PORT_ID));
                        return B_BAD_PORT_ID;
                }
        }
 
        // determine tail & get the length of the message
-       port_message* message = sPorts[slot].messages.Head();
+       port_message* message = port->messages.Head();
        if (message == NULL) {
-               panic("port %ld: no messages found\n", sPorts[slot].id);
+               panic("port %ld: no messages found\n", port->id);
                return B_ERROR;
        }
 
@@ -1112,10 +1166,10 @@
        info->sender_group = message->sender_group;
        info->sender_team = message->sender_team;
 
-       T(Info(sPorts[slot], message->code, B_OK));
+       T(Info(id, id->read_count, id->write_count, message->code, B_OK));
 
        // notify next one, as we haven't read from the port
-       sPorts[slot].read_condition.NotifyOne();
+       port->read_condition.NotifyOne();
 
        return B_OK;
 }
@@ -1127,17 +1181,16 @@
        if (!sPortsActive || id < 0)
                return B_BAD_PORT_ID;
 
-       int32 slot = id % sMaxPorts;
-
-       MutexLocker locker(sPorts[slot].lock);
-
-       if (sPorts[slot].id != id) {
+       // get the port
+       Port* port = get_locked_port(id);
+       if (port == NULL) {
                TRACE(("port_count: invalid port_id %ld\n", id));
                return B_BAD_PORT_ID;
        }
+       MutexLocker locker(port->lock, true);
 
        // return count of messages
-       return sPorts[slot].read_count;
+       return port->read_count;
 }

[... truncated: 275 lines follow ...]

Other related posts:

  • » [haiku-commits] r40235 - in haiku/branches/developer/bonefish/signals: headers/private/kernel src/system/kernel - ingo_weinhold