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
---
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(®->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(®->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(®->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(®->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
