[systemd-devel] [PATCH] names: take the registry write lock in kdbus_name_release()

[Djalal Harouni]

Take the write lock in kdbus_name_release() instead of
kdbus_cmd_name_release() in order to reduce the lock hold time.

This change permits to convert the kdbus_bus_find_conn_by_id() call to
kdbus_conn_find_peer() since the bus lock will also be taken later in
kdbus_name_release().

Another advantage is that now kdbus_cmd_name_release() and
kdbus_name_release() have the same semantic of kdbus_cmd_name_acquire()
and kdbus_name_acquire()

Signed-off-by: Djalal Harouni <tix...@opendz.org>
---
 names.c | 101 +++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 52 insertions(+), 49 deletions(-)

diff --git a/names.c b/names.c
index 3c98f46..70407d2 100644
--- a/names.c
+++ b/names.c
@@ -237,28 +237,56 @@ exit_release:
        return 0;
 }
 
-static int kdbus_name_release(struct kdbus_name_entry *e,
-                             struct kdbus_conn *conn)
+static int kdbus_name_release(struct kdbus_name_registry *reg,
+                             struct kdbus_conn *conn,
+                             const char *name)
 {
        struct kdbus_name_queue_item *q_tmp, *q;
+       struct kdbus_name_entry *e = NULL;
+       u32 hash;
+       int ret = 0;
+
+       hash = kdbus_str_hash(name);
+
+       /* lock order: domain -> bus -> ep -> names -> connection */
+       mutex_lock(&conn->bus->lock);
+       down_write(&reg->rwlock);
+
+       e = __kdbus_name_lookup(reg, hash, name);
+       if (!e) {
+               ret = -ESRCH;
+               goto exit_unlock;
+       }
 
        /* Is the connection already the real owner of the name? */
-       if (e->conn == conn)
-               return kdbus_name_entry_release(e, conn->bus);
-
-       /*
-        * Otherwise, walk the list of queued entries and search for
-        * items for the connection.
-        */
-       list_for_each_entry_safe(q, q_tmp, &e->queue_list, entry_entry) {
-               if (q->conn != conn)
-                       continue;
-               kdbus_name_queue_item_free(q);
-               return 0;
+       if (e->conn == conn) {
+               ret = kdbus_name_entry_release(e, conn->bus);
+       } else {
+               /*
+                * Otherwise, walk the list of queued entries and search
+                * for items for connection.
+                */
+
+               /* In case the name belongs to somebody else */
+               ret = -EADDRINUSE;
+
+               list_for_each_entry_safe(q, q_tmp,
+                                        &e->queue_list,
+                                        entry_entry) {
+                       if (q->conn != conn)
+                               continue;
+
+                       kdbus_name_queue_item_free(q);
+                       ret = 0;
+                       break;
+               }
        }
 
-       /* the name belongs to somebody else */
-       return -EADDRINUSE;
+exit_unlock:
+       up_write(&reg->rwlock);
+       mutex_unlock(&conn->bus->lock);
+
+       return ret;
 }
 
 /**
@@ -675,52 +703,27 @@ int kdbus_cmd_name_release(struct kdbus_name_registry 
*reg,
                           const struct kdbus_cmd_name *cmd)
 {
        struct kdbus_bus *bus = conn->bus;
-       struct kdbus_name_entry *e;
-       u32 hash;
        int ret = 0;
 
        if (!kdbus_name_is_valid(cmd->name, false))
                return -EINVAL;
 
-       hash = kdbus_str_hash(cmd->name);
-
-       /* lock order: domain -> bus -> ep -> names -> connection */
-       mutex_lock(&bus->lock);
-       down_write(&reg->rwlock);
-
-       e = __kdbus_name_lookup(reg, hash, cmd->name);
-       if (!e) {
-               ret = -ESRCH;
-               conn = NULL;
-               goto exit_unlock;
-       }
-
        /* privileged users can act on behalf of someone else */
        if (cmd->owner_id > 0) {
-               if (!kdbus_bus_uid_is_privileged(bus)) {
-                       ret = -EPERM;
-                       goto exit_unlock;
-               }
+               if (!kdbus_bus_uid_is_privileged(bus))
+                       return -EPERM;
 
-               conn = kdbus_bus_find_conn_by_id(bus, cmd->owner_id);
-               if (!conn) {
-                       ret = -ENXIO;
-                       goto exit_unlock;
-               }
+               conn = kdbus_conn_find_peer(conn, cmd->owner_id);
+               if (!conn)
+                       return -ENXIO;
        } else {
                kdbus_conn_ref(conn);
        }
 
-       ret = kdbus_name_release(e, conn);
+       ret = kdbus_name_release(reg, conn, cmd->name);
 
-exit_unlock:
-       up_write(&reg->rwlock);
-       mutex_unlock(&bus->lock);
-
-       if (conn) {
-               kdbus_notify_flush(conn->bus);
-               kdbus_conn_unref(conn);
-       }
+       kdbus_notify_flush(conn->bus);
+       kdbus_conn_unref(conn);
 
        return ret;
 }
-- 
1.9.0

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel