[PATCH 2/4] kdbus: inform caller about exact updates on NAME_ACQUIRE

From: Daniel Mack
Date: Fri Aug 07 2015 - 10:37:55 EST


From: David Herrmann <dh.herrmann@xxxxxxxxx>

This adds two new return flags for KDBUS_CMD_NAME_ACQUIRE:

* The KDBUS_NAME_PRIMARY flag is set for a name if, and only if, the
connection is currently the primary owner of a name. It is thus the
negation of KDBUS_NAME_IN_QUEUE, but is required to distinguish the
case from the situation where the connection is neither queued nor the
primary owner of a name.
Additionally, this flag is included in name listings and events.

* The KDBUS_NAME_ACQUIRED flag is exclusively used as return flag for
KDBUS_CMD_NAME_ACQUIRE to let the caller know whether _this_ exact
call actually queued the connection on the name. If the flag is not
set, the connection was either already queued and only the flags were
updated, or the connection is not queued at all.

This information was previously available to the caller via error-codes
from KDBUS_CMD_NAME_ACQUIRE. However, in some situations we actually
modify kernel state but return an error. This is considered bad style and
we really need to avoid this. Hence, these two new flags allow us to
avoid returning errors, but still inform the caller about the exact
conditions of the execution.

Reviewed-by: Daniel Mack <daniel@xxxxxxxxxx>
Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
---
include/uapi/linux/kdbus.h | 4 ++++
ipc/kdbus/names.c | 38 ++++++++++++++++++++++++++++----------
2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/kdbus.h b/include/uapi/linux/kdbus.h
index ecffc6b..4fc44cb 100644
--- a/include/uapi/linux/kdbus.h
+++ b/include/uapi/linux/kdbus.h
@@ -854,6 +854,8 @@ enum kdbus_make_flags {
* @KDBUS_NAME_QUEUE: Name should be queued if busy
* @KDBUS_NAME_IN_QUEUE: Name is queued
* @KDBUS_NAME_ACTIVATOR: Name is owned by a activator connection
+ * @KDBUS_NAME_PRIMARY: Primary owner of the name
+ * @KDBUS_NAME_ACQUIRED: Name was acquired/queued _now_
*/
enum kdbus_name_flags {
KDBUS_NAME_REPLACE_EXISTING = 1ULL << 0,
@@ -861,6 +863,8 @@ enum kdbus_name_flags {
KDBUS_NAME_QUEUE = 1ULL << 2,
KDBUS_NAME_IN_QUEUE = 1ULL << 3,
KDBUS_NAME_ACTIVATOR = 1ULL << 4,
+ KDBUS_NAME_PRIMARY = 1ULL << 5,
+ KDBUS_NAME_ACQUIRED = 1ULL << 6,
};

/**
diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index 7a6e61c..a47ee54 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -211,7 +211,8 @@ kdbus_name_lookup_unlocked(struct kdbus_name_registry *reg, const char *name)
return kdbus_name_entry_find(reg, kdbus_strhash(name), name);
}

-static int kdbus_name_become_activator(struct kdbus_name_owner *owner)
+static int kdbus_name_become_activator(struct kdbus_name_owner *owner,
+ u64 *return_flags)
{
if (kdbus_name_owner_is_used(owner))
return -EALREADY;
@@ -221,23 +222,30 @@ static int kdbus_name_become_activator(struct kdbus_name_owner *owner)
owner->name->activator = owner;
owner->flags |= KDBUS_NAME_ACTIVATOR;

- if (kdbus_name_entry_first(owner->name))
+ if (kdbus_name_entry_first(owner->name)) {
owner->flags |= KDBUS_NAME_IN_QUEUE;
- else
+ } else {
+ owner->flags |= KDBUS_NAME_PRIMARY;
kdbus_notify_name_change(owner->conn->ep->bus,
KDBUS_ITEM_NAME_ADD,
0, owner->conn->id,
0, owner->flags,
owner->name->name);
+ }
+
+ if (return_flags)
+ *return_flags = owner->flags | KDBUS_NAME_ACQUIRED;

return 0;
}

-static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
+static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags,
+ u64 *return_flags)
{
struct kdbus_name_owner *primary, *activator;
struct kdbus_name_entry *name;
struct kdbus_bus *bus;
+ u64 nflags = 0;
int ret = 0;

name = owner->name;
@@ -259,6 +267,8 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
*/

list_add(&owner->name_entry, &name->queue);
+ owner->flags |= KDBUS_NAME_PRIMARY;
+ nflags |= KDBUS_NAME_ACQUIRED;

/* move messages to new owner on activation */
if (activator) {
@@ -268,6 +278,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
activator->conn->id, owner->conn->id,
activator->flags, owner->flags,
name->name);
+ activator->flags &= ~KDBUS_NAME_PRIMARY;
activator->flags |= KDBUS_NAME_IN_QUEUE;
} else {
kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_ADD,
@@ -283,6 +294,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
* For compatibility, we have to return -EALREADY.
*/

+ owner->flags |= KDBUS_NAME_PRIMARY;
ret = -EALREADY;

} else if ((primary->flags & KDBUS_NAME_ALLOW_REPLACEMENT) &&
@@ -295,6 +307,8 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)

list_del_init(&owner->name_entry);
list_add(&owner->name_entry, &name->queue);
+ owner->flags |= KDBUS_NAME_PRIMARY;
+ nflags |= KDBUS_NAME_ACQUIRED;

kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_CHANGE,
primary->conn->id, owner->conn->id,
@@ -303,6 +317,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)

/* requeue old primary, or drop if queueing not wanted */
if (primary->flags & KDBUS_NAME_QUEUE) {
+ primary->flags &= ~KDBUS_NAME_PRIMARY;
primary->flags |= KDBUS_NAME_IN_QUEUE;
} else {
list_del_init(&primary->name_entry);
@@ -317,8 +332,10 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
*/

owner->flags |= KDBUS_NAME_IN_QUEUE;
- if (!kdbus_name_owner_is_used(owner))
+ if (!kdbus_name_owner_is_used(owner)) {
list_add_tail(&owner->name_entry, &name->queue);
+ nflags |= KDBUS_NAME_ACQUIRED;
+ }
} else if (kdbus_name_owner_is_used(owner)) {
/*
* Already queued on name, but re-queueing was not requested.
@@ -337,6 +354,9 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
ret = -EEXIST;
}

+ if (return_flags)
+ *return_flags = owner->flags | nflags;
+
return ret;
}

@@ -392,15 +412,12 @@ int kdbus_name_acquire(struct kdbus_name_registry *reg,
}

if (flags & KDBUS_NAME_ACTIVATOR)
- ret = kdbus_name_become_activator(owner);
+ ret = kdbus_name_become_activator(owner, return_flags);
else
- ret = kdbus_name_update(owner, flags);
+ ret = kdbus_name_update(owner, flags, return_flags);
if (ret < 0)
goto exit;

- if (return_flags)
- *return_flags = owner->flags;
-
exit:
if (owner && !kdbus_name_owner_is_used(owner))
kdbus_name_owner_free(owner);
@@ -431,6 +448,7 @@ static void kdbus_name_release_unlocked(struct kdbus_name_owner *owner)
if (next) {
/* hand to next in queue */
next->flags &= ~KDBUS_NAME_IN_QUEUE;
+ next->flags |= KDBUS_NAME_PRIMARY;
if (next == name->activator)
kdbus_conn_move_messages(next->conn,
owner->conn,
--
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/