mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Daniel Mack <daniel@zonque.org>
To: gregkh@linuxfoundation.org
Cc: dh.herrmann@gmail.com, tixxdz@opendz.org, teg@jklm.no,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/4] kdbus: inform caller about exact updates on NAME_ACQUIRE
Date: Fri,  7 Aug 2015 16:36:34 +0200	[thread overview]
Message-ID: <1438958196-23036-3-git-send-email-daniel@zonque.org> (raw)
In-Reply-To: <1438958196-23036-1-git-send-email-daniel@zonque.org>

From: David Herrmann <dh.herrmann@gmail.com>

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@zonque.org>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 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


  parent reply	other threads:[~2015-08-07 14:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-07 14:36 [PATCH 0/4] kdbus: name registry cleanup patches Daniel Mack
2015-08-07 14:36 ` [PATCH 1/4] kdbus: restructure name-registry to follow dbus-spec Daniel Mack
2015-08-07 14:36 ` Daniel Mack [this message]
2015-08-07 14:36 ` [PATCH 3/4] kdbus: never return <0 from ioctls if we changed state Daniel Mack
2015-08-07 14:36 ` [PATCH 4/4] kdbus: selftests: add more name registry tests Daniel Mack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1438958196-23036-3-git-send-email-daniel@zonque.org \
    --to=daniel@zonque.org \
    --cc=dh.herrmann@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=teg@jklm.no \
    --cc=tixxdz@opendz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®