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