Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1242233 > unrolled thread

[PATCH 00/44] kdbus cleanups

Started bySergei Zviagintsev <sergei@s15v.net>
First post2015-10-08 13:40 +0200
Last post2015-10-09 09:30 +0200
Articles 20 on this page of 61 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 00/44] kdbus cleanups Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 32/44] kdbus: Remove duplicated code from kdbus_conn_lock2() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 16/44] kdbus: Drop redundant code from kdbus_name_acquire() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 42/44] kdbus: Check if fd is allocated before trying to free it Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 37/44] kdbus: Fix error path in kdbus_meta_proc_collect_cgroup() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 34/44] kdbus: Improve kdbus_conn_entry_sync_attach() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 39/44] kdbus: Cleanup kdbus_user_lookup() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 10/44] kdbus: Use conditional operator Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 01/44] Documentation/kdbus: Document new name registry flags Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:40 +0200
    [PATCH 12/44] kdbus: Use conventional list macros in __kdbus_pool_slice_release() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:30 +0200
        Re: [PATCH 15/44] kdbus: Simplify bitwise expression in  kdbus_meta_get_mask() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:00 +0200
    [PATCH 08/44] kdbus: Rename var in kdbus_meta_export_caps() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 41/44] kdbus: Fix memfd install algorithm Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 27/44] kdbus: Cleanup kdbus_conn_new() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 20/44] kdbus: Drop useless initialization from kdbus_cmd_hello() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 22/44] kdbus: Cleanup error path in kdbus_staging_new_user() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 05/44] kdbus: Add comment on merging free pool slices Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 05/44] kdbus: Add comment on merging free pool slices David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:00 +0200
        Re: [PATCH 05/44] kdbus: Add comment on merging free pool slices Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:20 +0200
    [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 02/44] uapi: kdbus.h: Kernel-doc fixes David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 15:50 +0200
    [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 30/44] kdbus: Cleanup kdbus_meta_proc_mask() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:50 +0200
    [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 14/44] kdbus: Simplify expression in kdbus_get_memfd() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:30 +0200
    [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 13/44] kdbus: Use list_next_entry() in kdbus_queue_entry_unlink() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:10 +0200
    [PATCH 33/44] kdbus: Improve kdbus_staging_reserve() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 35/44] kdbus: Drop goto from kdbus_queue_entry_link() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
    [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:40 +0200
      Re: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 17:00 +0200
        Re: [PATCH 31/44] kdbus: Cleanup kdbus_conn_move_messages() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:50 +0200
    [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 21/44] kdbus: Cleanup tests in kdbus_cmd_send() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:10 +0200
    [PATCH 11/44] kdbus: Cosmetic fix of kdbus_name_is_valid() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 23/44] kdbus: Cleanup kdbus_conn_call() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:20 +0200
    [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 03/44] kdbus: Kernel-docs and comments trivial fixes David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 15:50 +0200
    [PATCH 28/44] kdbus: Cleanup kdbus_queue_entry_new() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:40 +0200
        Re: [PATCH 25/44] kdbus: Cleanup kdbus_cmd_conn_info() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:50 +0200
    [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:50 +0200
        Re: [PATCH 26/44] kdbus: Cleanup kdbus_pin_dst() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:50 +0200
    [PATCH 09/44] kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 19/44] kdbus: Drop useless initialization from kdbus_conn_reply() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
      Re: [PATCH 18/44] kdbus: Add var initialization to kdbus_conn_entry_insert() David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 16:30 +0200
        Re: [PATCH 18/44] kdbus: Add var initialization to  kdbus_conn_entry_insert() Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 20:00 +0200
    [PATCH 06/44] kdbus: Fix kernel-doc for struct kdbus_gaps Sergei Zviagintsev <sergei@s15v.net> - 2015-10-08 13:50 +0200
    Re: [PATCH 00/44] kdbus cleanups David Herrmann <dh.herrmann@gmail.com> - 2015-10-08 17:30 +0200
      Re: [PATCH 00/44] kdbus cleanups Sergei Zviagintsev <sergei@s15v.net> - 2015-10-09 09:30 +0200

Page 1 of 4  [1] 2 3 4  Next page →


#1242233 — [PATCH 00/44] kdbus cleanups

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 00/44] kdbus cleanups
Message-ID<qhhId-2d4-3@gated-at.bofh.it>
Hi all,

This is a set of various kdbus code cleanups. Patches are ordered by
increasing complexity, starting with docs and comments fixes and
one-liners.

Patch 29 is the revised version of
http://lkml.kernel.org/g/1435497454-10464-6-git-send-email-sergei@s15v.net

Feel free to ask to change layout of this, split/join, etc if necessary.

Thanks, Sergei

Sergei Zviagintsev (44):
  Documentation/kdbus: Document new name registry flags
  uapi: kdbus.h: Kernel-doc fixes
  kdbus: Kernel-docs and comments trivial fixes
  kdbus: Update kernel-doc for struct kdbus_pool
  kdbus: Add comment on merging free pool slices
  kdbus: Fix kernel-doc for struct kdbus_gaps
  kdbus: Fix comment on translation of caps between namespaces
  kdbus: Rename var in kdbus_meta_export_caps()
  kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant
  kdbus: Use conditional operator
  kdbus: Cosmetic fix of kdbus_name_is_valid()
  kdbus: Use conventional list macros in __kdbus_pool_slice_release()
  kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()
  kdbus: Simplify expression in kdbus_get_memfd()
  kdbus: Simplify bitwise expression in kdbus_meta_get_mask()
  kdbus: Drop redundant code from kdbus_name_acquire()
  kdbus: Drop duplicated code from kdbus_pool_slice_alloc()
  kdbus: Add var initialization to kdbus_conn_entry_insert()
  kdbus: Drop useless initialization from kdbus_conn_reply()
  kdbus: Drop useless initialization from kdbus_cmd_hello()
  kdbus: Cleanup tests in kdbus_cmd_send()
  kdbus: Cleanup error path in kdbus_staging_new_user()
  kdbus: Cleanup kdbus_conn_call()
  kdbus: Cleanup kdbus_conn_unicast()
  kdbus: Cleanup kdbus_cmd_conn_info()
  kdbus: Cleanup kdbus_pin_dst()
  kdbus: Cleanup kdbus_conn_new()
  kdbus: Cleanup kdbus_queue_entry_new()
  kdbus: Improve tests on incrementing quota
  kdbus: Cleanup kdbus_meta_proc_mask()
  kdbus: Cleanup kdbus_conn_move_messages()
  kdbus: Remove duplicated code from kdbus_conn_lock2()
  kdbus: Improve kdbus_staging_reserve()
  kdbus: Improve kdbus_conn_entry_sync_attach()
  kdbus: Drop goto from kdbus_queue_entry_link()
  kdbus: Improve kdbus_name_release()
  kdbus: Fix error path in kdbus_meta_proc_collect_cgroup()
  kdbus: Fix error path in kdbus_user_lookup()
  kdbus: Cleanup kdbus_user_lookup()
  kdbus: Cleanup kdbus_item_validate_name()
  kdbus: Fix memfd install algorithm
  kdbus: Check if fd is allocated before trying to free it
  kdbus: Give up on failed fd allocation
  kdbus: Cleanup kdbus_gaps_install()

 Documentation/kdbus/kdbus.name.xml |  42 +++++++++-
 include/uapi/linux/kdbus.h         |  43 +++++-----
 ipc/kdbus/connection.c             | 157 +++++++++++++++----------------------
 ipc/kdbus/connection.h             |  19 ++---
 ipc/kdbus/domain.c                 |  38 +++++----
 ipc/kdbus/fs.c                     |   2 +-
 ipc/kdbus/item.c                   |  26 +++---
 ipc/kdbus/limits.h                 |   3 -
 ipc/kdbus/message.c                |  81 +++++++++----------
 ipc/kdbus/message.h                |   9 ++-
 ipc/kdbus/metadata.c               |  79 ++++++++++---------
 ipc/kdbus/names.c                  |  32 ++++----
 ipc/kdbus/node.c                   |   4 +-
 ipc/kdbus/pool.c                   |  26 +++---
 ipc/kdbus/queue.c                  |  51 ++++++------
 ipc/kdbus/queue.h                  |   2 +-
 16 files changed, 298 insertions(+), 316 deletions(-)

-- 
1.8.3.1

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

[toc] | [next] | [standalone]


#1242235 — [PATCH 32/44] kdbus: Remove duplicated code from kdbus_conn_lock2()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 32/44] kdbus: Remove duplicated code from kdbus_conn_lock2()
Message-ID<qhhIe-2d4-27@gated-at.bofh.it>
In reply to#1242233
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.h | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index 679f393d3e68..3b382b604348 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -215,17 +215,13 @@ static inline int kdbus_conn_is_monitor(const struct kdbus_conn *conn)
  */
 static inline void kdbus_conn_lock2(struct kdbus_conn *a, struct kdbus_conn *b)
 {
-	if (a < b) {
-		if (a)
-			mutex_lock(&a->lock);
-		if (b && b != a)
-			mutex_lock_nested(&b->lock, !!a);
-	} else {
-		if (b)
-			mutex_lock(&b->lock);
-		if (a && a != b)
-			mutex_lock_nested(&a->lock, !!b);
-	}
+	struct kdbus_conn *lo = min(a, b);
+	struct kdbus_conn *hi = max(a, b);
+
+	if (lo)
+		mutex_lock(&lo->lock);
+	if (hi && hi != lo)
+		mutex_lock_nested(&hi->lock, !!lo);
 }
 
 /**
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242236 — [PATCH 16/44] kdbus: Drop redundant code from kdbus_name_acquire()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 16/44] kdbus: Drop redundant code from kdbus_name_acquire()
Message-ID<qhhIe-2d4-31@gated-at.bofh.it>
In reply to#1242233
We already reached the end of function at this point, so remove useless
goto.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/names.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index 4dea83defc8e..a1c3442f8e53 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -412,8 +412,6 @@ int kdbus_name_acquire(struct kdbus_name_registry *reg,
 		ret = kdbus_name_become_activator(owner, return_flags);
 	else
 		ret = kdbus_name_update(owner, flags, return_flags);
-	if (ret < 0)
-		goto exit;
 
 exit:
 	if (owner && !kdbus_name_owner_is_used(owner))
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242237 — [PATCH 42/44] kdbus: Check if fd is allocated before trying to free it

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 42/44] kdbus: Check if fd is allocated before trying to free it
Message-ID<qhhIe-2d4-37@gated-at.bofh.it>
In reply to#1242233
Elements of `fds' array were set to -1 in case if we couldn't allocate
a fd. Verify that element contains a valid fd number before submitting
it to put_unused_fd().

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/message.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index 0653a085c104..da685049d66c 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -221,7 +221,8 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 exit:
 	if (ret < 0)
 		for (i = 0; i < n_fds; ++i)
-			put_unused_fd(fds[i]);
+			if (fds[i] >= 0)
+				put_unused_fd(fds[i]);
 	kfree(fds);
 	*out_incomplete = incomplete_fds;
 	return ret;
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242238 — [PATCH 37/44] kdbus: Fix error path in kdbus_meta_proc_collect_cgroup()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 37/44] kdbus: Fix error path in kdbus_meta_proc_collect_cgroup()
Message-ID<qhhIe-2d4-35@gated-at.bofh.it>
In reply to#1242233
Current code checks return value of task_cgroup_path(), which can be
NULL if provided buffer isn't long enough to store path there, but
alters mp->valid in case of error, producing inconsistency. Return
-ENAMETOOLONG if task_cgroup_path() fails.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/metadata.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index b8d094d9fb56..f4f2b1af81a7 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -269,12 +269,15 @@ static int kdbus_meta_proc_collect_cgroup(struct kdbus_meta_proc *mp)
 		return -ENOMEM;
 
 	s = task_cgroup_path(current, page, PAGE_SIZE);
-	if (s) {
-		mp->cgroup = kstrdup(s, GFP_KERNEL);
-		if (!mp->cgroup) {
-			free_page((unsigned long)page);
-			return -ENOMEM;
-		}
+	if (!s) {
+		free_page((unsigned long)page);
+		return -ENAMETOOLONG;
+	}
+
+	mp->cgroup = kstrdup(s, GFP_KERNEL);
+	if (!mp->cgroup) {
+		free_page((unsigned long)page);
+		return -ENOMEM;
 	}
 
 	free_page((unsigned long)page);
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242239 — [PATCH 34/44] kdbus: Improve kdbus_conn_entry_sync_attach()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 34/44] kdbus: Improve kdbus_conn_entry_sync_attach()
Message-ID<qhhIe-2d4-29@gated-at.bofh.it>
In reply to#1242233
Use goto to handle error paths in conventional way. Use conditional
operator instead of `remote_ret' var. Update the comment on waking up
remote peer.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 52 ++++++++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 31 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 6ee688d3de53..081f248339f5 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -820,47 +820,37 @@ static int kdbus_conn_entry_sync_attach(struct kdbus_conn *conn_dst,
 					struct kdbus_reply *reply_wake)
 {
 	struct kdbus_queue_entry *entry;
-	int remote_ret, ret = 0;
+	int ret = 0;
 
 	mutex_lock(&reply_wake->reply_dst->lock);
 
-	/*
-	 * If we are still waiting then proceed, allocate a queue
-	 * entry and attach it to the reply object
-	 */
-	if (reply_wake->waiting) {
-		entry = kdbus_conn_entry_make(reply_wake->reply_src, conn_dst,
-					      staging);
-		if (IS_ERR(entry))
-			ret = PTR_ERR(entry);
-		else
-			/* Attach the entry to the reply object */
-			reply_wake->queue_entry = entry;
-	} else {
+	if (!reply_wake->waiting) {
 		ret = -ECONNRESET;
+		goto wake_up_remote;
 	}
 
 	/*
-	 * Update the reply object and wake up remote peer only
-	 * on appropriate return codes
-	 *
-	 * * -ECOMM: if the replying connection failed with -ECOMM
-	 *           then wakeup remote peer with -EREMOTEIO
-	 *
-	 *           We do this to differenciate between -ECOMM errors
-	 *           from the original sender perspective:
-	 *           -ECOMM error during the sync send and
-	 *           -ECOMM error during the sync reply, this last
-	 *           one is rewritten to -EREMOTEIO
-	 *
-	 * * Wake up on all other return codes.
+	 * We are still waiting. Allocate a queue entry and attach it to the
+	 * reply object.
 	 */
-	remote_ret = ret;
+	entry = kdbus_conn_entry_make(reply_wake->reply_src, conn_dst, staging);
+	if (IS_ERR(entry)) {
+		ret = PTR_ERR(entry);
+		goto wake_up_remote;
+	}
 
-	if (ret == -ECOMM)
-		remote_ret = -EREMOTEIO;
+	reply_wake->queue_entry = entry;
 
-	kdbus_sync_reply_wakeup(reply_wake, remote_ret);
+	/*
+	 * If the replying connection failed with -ECOMM then wakeup remote peer
+	 * with -EREMOTEIO. We do this to differentiate between -ECOMM errors
+	 * from the original sender perspective:
+	 *   * -ECOMM error during the sync send and
+	 *   * -ECOMM error during the sync reply, this last one is rewritten
+	 *     to -EREMOTEIO
+	 */
+wake_up_remote:
+	kdbus_sync_reply_wakeup(reply_wake, (ret == -ECOMM) ? -EREMOTEIO : ret);
 	kdbus_reply_unlink(reply_wake);
 	mutex_unlock(&reply_wake->reply_dst->lock);
 
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242240 — [PATCH 39/44] kdbus: Cleanup kdbus_user_lookup()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 39/44] kdbus: Cleanup kdbus_user_lookup()
Message-ID<qhhIe-2d4-39@gated-at.bofh.it>
In reply to#1242233
 - Do not initialize `u' with NULL as value is assigned to it at the
   first use.

 - Simplify if-statement. If `old' is non-NULL, it means that provided
   @uid is valid, so there is no need to check both.

 - Use `uid' and `domain' instead of `u->uid' and `u->domain' in error
   path, which is equivalent but more concise and readable (as we used
   same vars in the code above).

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/domain.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
index 31cd09fb572f..1da893089889 100644
--- a/ipc/kdbus/domain.c
+++ b/ipc/kdbus/domain.c
@@ -188,7 +188,7 @@ int kdbus_domain_populate(struct kdbus_domain *domain, unsigned int access)
  */
 struct kdbus_user *kdbus_user_lookup(struct kdbus_domain *domain, kuid_t uid)
 {
-	struct kdbus_user *u = NULL, *old = NULL;
+	struct kdbus_user *u, *old = NULL;
 	int ret;
 
 	mutex_lock(&domain->lock);
@@ -217,16 +217,14 @@ struct kdbus_user *kdbus_user_lookup(struct kdbus_domain *domain, kuid_t uid)
 	atomic_set(&u->buses, 0);
 	atomic_set(&u->connections, 0);
 
-	if (uid_valid(uid)) {
-		if (old) {
-			idr_replace(&domain->user_idr, u, __kuid_val(uid));
-			old->uid = INVALID_UID; /* mark old as removed */
-		} else {
-			ret = idr_alloc(&domain->user_idr, u, __kuid_val(uid),
-					__kuid_val(uid) + 1, GFP_KERNEL);
-			if (ret < 0)
-				goto exit_free;
-		}
+	if (old) {
+		idr_replace(&domain->user_idr, u, __kuid_val(uid));
+		old->uid = INVALID_UID; /* mark old as removed */
+	} else if (uid_valid(uid)) {
+		ret = idr_alloc(&domain->user_idr, u, __kuid_val(uid),
+				__kuid_val(uid) + 1, GFP_KERNEL);
+		if (ret < 0)
+			goto exit_free;
 	}
 
 	/*
@@ -242,10 +240,10 @@ struct kdbus_user *kdbus_user_lookup(struct kdbus_domain *domain, kuid_t uid)
 	return u;
 
 exit_idr:
-	if (uid_valid(u->uid))
-		idr_remove(&domain->user_idr, __kuid_val(u->uid));
+	if (uid_valid(uid))
+		idr_remove(&domain->user_idr, __kuid_val(uid));
 exit_free:
-	kdbus_domain_unref(u->domain);
+	kdbus_domain_unref(domain);
 	kfree(u);
 exit_unlock:
 	mutex_unlock(&domain->lock);
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242241 — [PATCH 10/44] kdbus: Use conditional operator

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 10/44] kdbus: Use conditional operator
Message-ID<qhhIf-2d4-41@gated-at.bofh.it>
In reply to#1242233
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/names.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index bf44ca3f12b6..6b31b38ac2ad 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -438,10 +438,7 @@ static void kdbus_name_release_unlocked(struct kdbus_name_owner *owner)
 		name->activator = NULL;
 
 	if (!primary || owner == primary) {
-		next = kdbus_name_entry_first(name);
-		if (!next)
-			next = name->activator;
-
+		next = kdbus_name_entry_first(name) ?: name->activator;
 		if (next) {
 			/* hand to next in queue */
 			next->flags &= ~KDBUS_NAME_IN_QUEUE;
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242242 — [PATCH 01/44] Documentation/kdbus: Document new name registry flags

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 01/44] Documentation/kdbus: Document new name registry flags
Message-ID<qhhIf-2d4-43@gated-at.bofh.it>
In reply to#1242233
Add description of KDBUS_NAME_PRIMARY and KDBUS_NAME_ACQUIRED flags
which were added in commit 0486b859f05a ("kdbus: inform caller about
exact updates on NAME_ACQUIRE").

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 Documentation/kdbus/kdbus.name.xml | 42 +++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/Documentation/kdbus/kdbus.name.xml b/Documentation/kdbus/kdbus.name.xml
index 3f5f6a6c5ed6..85c8c30c0edd 100644
--- a/Documentation/kdbus/kdbus.name.xml
+++ b/Documentation/kdbus/kdbus.name.xml
@@ -209,6 +209,32 @@ struct kdbus_cmd {
                 </para>
               </listitem>
             </varlistentry>
+
+            <varlistentry>
+              <term><constant>KDBUS_NAME_PRIMARY</constant></term>
+              <listitem>
+                <para>
+		  The connection is currently the primary owner of the name.
+		  This flag is the negation of
+		  <constant>KDBUS_NAME_IN_QUEUE</constant>, but is required to
+		  distinguish the case from the situation where the connection
+		  is neither queued nor the primary owner of the name.
+                </para>
+              </listitem>
+            </varlistentry>
+
+            <varlistentry>
+              <term><constant>KDBUS_NAME_ACQUIRED</constant></term>
+              <listitem>
+                <para>
+                  This flag is used to let the caller know whether
+		  <emphasis>this</emphasis> 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.
+                </para>
+              </listitem>
+            </varlistentry>
           </variablelist>
         </listitem>
       </varlistentry>
@@ -489,9 +515,19 @@ struct kdbus_info {
               <listitem>
                 <para>
                   When retrieving a list of currently acquired names in the
-                  registry, this flag indicates whether the connection
-                  actually owns the name or is currently waiting for it to
-                  become available.
+                  registry, this flag indicates that the connection is currently
+                  waiting for the name to become available.
+                </para>
+              </listitem>
+            </varlistentry>
+
+            <varlistentry>
+              <term><constant>KDBUS_NAME_PRIMARY</constant></term>
+              <listitem>
+                <para>
+                  When retrieving a list of currently acquired names in the
+                  registry, this flag indicates that the connection is the
+                  primary owner of the name.
                 </para>
               </listitem>
             </varlistentry>
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242243 — [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast()
Message-ID<qhhIf-2d4-45@gated-at.bofh.it>
In reply to#1242233
Do not initialize `name' and `ret' as values are assigned to them at
the first use by kdbus_pin_dst(). Simplify handling of
kdbus_conn_entry_insert() return value and drop useless goto.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index db49f282a1bf..b3c5f20a57d8 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -1229,12 +1229,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
 			      struct kdbus_staging *staging)
 {
 	const struct kdbus_msg *msg = staging->msg;
-	struct kdbus_name_entry *name = NULL;
+	struct kdbus_name_entry *name;
 	struct kdbus_reply *wait = NULL;
 	struct kdbus_conn *dst = NULL;
 	struct kdbus_bus *bus = src->ep->bus;
 	bool is_signal = (msg->flags & KDBUS_MSG_SIGNAL);
-	int ret = 0;
+	int ret;
 
 	if (WARN_ON(msg->dst_id == KDBUS_DST_ID_BROADCAST) ||
 	    WARN_ON(!(msg->flags & KDBUS_MSG_EXPECT_REPLY) &&
@@ -1245,7 +1245,6 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
 	down_read(&bus->name_registry->rwlock);
 
 	/* find and pin destination */
-
 	ret = kdbus_pin_dst(bus, staging, &name, &dst);
 	if (ret < 0)
 		goto exit;
@@ -1276,11 +1275,10 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
 		kdbus_bus_eavesdrop(bus, src, staging);
 
 	ret = kdbus_conn_entry_insert(src, dst, staging, wait, name);
-	if (ret < 0 && !is_signal)
-		goto exit;
 
 	/* signals are treated like broadcasts, recv-errors are ignored */
-	ret = 0;
+	if (is_signal)
+		ret = 0;
 
 exit:
 	up_read(&bus->name_registry->rwlock);
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242438 — Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:40 +0200
SubjectRe: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast()
Message-ID<qhkwq-6iy-31@gated-at.bofh.it>
In reply to#1242243
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Do not initialize `name' and `ret' as values are assigned to them at
> the first use by kdbus_pin_dst(). Simplify handling of
> kdbus_conn_entry_insert() return value and drop useless goto.
>
> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/connection.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index db49f282a1bf..b3c5f20a57d8 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -1229,12 +1229,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
>                               struct kdbus_staging *staging)
>  {
>         const struct kdbus_msg *msg = staging->msg;
> -       struct kdbus_name_entry *name = NULL;
> +       struct kdbus_name_entry *name;
>         struct kdbus_reply *wait = NULL;
>         struct kdbus_conn *dst = NULL;
>         struct kdbus_bus *bus = src->ep->bus;
>         bool is_signal = (msg->flags & KDBUS_MSG_SIGNAL);
> -       int ret = 0;
> +       int ret;
>
>         if (WARN_ON(msg->dst_id == KDBUS_DST_ID_BROADCAST) ||
>             WARN_ON(!(msg->flags & KDBUS_MSG_EXPECT_REPLY) &&
> @@ -1245,7 +1245,6 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
>         down_read(&bus->name_registry->rwlock);
>
>         /* find and pin destination */
> -

If a comment is addressed to a whole following block, we usually put a
newline after it. Only if the comment is only addressed at the next
code-line, we don't.

>         ret = kdbus_pin_dst(bus, staging, &name, &dst);
>         if (ret < 0)
>                 goto exit;
> @@ -1276,11 +1275,10 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
>                 kdbus_bus_eavesdrop(bus, src, staging);
>
>         ret = kdbus_conn_entry_insert(src, dst, staging, wait, name);
> -       if (ret < 0 && !is_signal)
> -               goto exit;
>
>         /* signals are treated like broadcasts, recv-errors are ignored */
> -       ret = 0;
> +       if (is_signal)
> +               ret = 0;

Why? Just to reduce the line-count? You break the code-flow here, by
making the success-path conditional, instead of the error-path.

Thanks
David

>
>  exit:
>         up_read(&bus->name_registry->rwlock);
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1243593 — Re: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:40 +0200
SubjectRe: [PATCH 24/44] kdbus: Cleanup kdbus_conn_unicast()
Message-ID<qhKKd-1RL-7@gated-at.bofh.it>
In reply to#1242438
On Thu, Oct 08, 2015 at 04:34:27PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> > Do not initialize `name' and `ret' as values are assigned to them at
> > the first use by kdbus_pin_dst(). Simplify handling of
> > kdbus_conn_entry_insert() return value and drop useless goto.
> >
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> >  ipc/kdbus/connection.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> > index db49f282a1bf..b3c5f20a57d8 100644
> > --- a/ipc/kdbus/connection.c
> > +++ b/ipc/kdbus/connection.c
> > @@ -1229,12 +1229,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
> >                               struct kdbus_staging *staging)
> >  {
> >         const struct kdbus_msg *msg = staging->msg;
> > -       struct kdbus_name_entry *name = NULL;
> > +       struct kdbus_name_entry *name;
> >         struct kdbus_reply *wait = NULL;
> >         struct kdbus_conn *dst = NULL;
> >         struct kdbus_bus *bus = src->ep->bus;
> >         bool is_signal = (msg->flags & KDBUS_MSG_SIGNAL);
> > -       int ret = 0;
> > +       int ret;
> >
> >         if (WARN_ON(msg->dst_id == KDBUS_DST_ID_BROADCAST) ||
> >             WARN_ON(!(msg->flags & KDBUS_MSG_EXPECT_REPLY) &&
> > @@ -1245,7 +1245,6 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
> >         down_read(&bus->name_registry->rwlock);
> >
> >         /* find and pin destination */
> > -
> 
> If a comment is addressed to a whole following block, we usually put a
> newline after it. Only if the comment is only addressed at the next
> code-line, we don't.

Sorry, this change is here by mistake and shouldn't be in the patch at
all.

> 
> >         ret = kdbus_pin_dst(bus, staging, &name, &dst);
> >         if (ret < 0)
> >                 goto exit;
> > @@ -1276,11 +1275,10 @@ static int kdbus_conn_unicast(struct kdbus_conn *src,
> >                 kdbus_bus_eavesdrop(bus, src, staging);
> >
> >         ret = kdbus_conn_entry_insert(src, dst, staging, wait, name);
> > -       if (ret < 0 && !is_signal)
> > -               goto exit;
> >
> >         /* signals are treated like broadcasts, recv-errors are ignored */
> > -       ret = 0;
> > +       if (is_signal)
> > +               ret = 0;
> 
> Why? Just to reduce the line-count? You break the code-flow here, by
> making the success-path conditional, instead of the error-path.

IMO, it's easier to read as it's exacly what the comment says: ignore an
error in the case of signal. But I don't mind omitting this change from
the next submission.

> 
> Thanks
> David
> 
> >
> >  exit:
> >         up_read(&bus->name_registry->rwlock);
> > --
> > 1.8.3.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1242244 — [PATCH 12/44] kdbus: Use conventional list macros in __kdbus_pool_slice_release()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 12/44] kdbus: Use conventional list macros in __kdbus_pool_slice_release()
Message-ID<qhhIf-2d4-49@gated-at.bofh.it>
In reply to#1242233
Use list_next_entry() and list_prev_entry().

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/pool.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ipc/kdbus/pool.c b/ipc/kdbus/pool.c
index c26ef963d8d1..cb692a35755b 100644
--- a/ipc/kdbus/pool.c
+++ b/ipc/kdbus/pool.c
@@ -287,8 +287,7 @@ static void __kdbus_pool_slice_release(struct kdbus_pool_slice *slice)
 	if (!list_is_last(&slice->entry, &pool->slices)) {
 		struct kdbus_pool_slice *s;
 
-		s = list_entry(slice->entry.next,
-			       struct kdbus_pool_slice, entry);
+		s = list_next_entry(slice, entry);
 		if (s->free) {
 			rb_erase(&s->rb_node, &pool->slices_free);
 			list_del(&s->entry);
@@ -301,8 +300,7 @@ static void __kdbus_pool_slice_release(struct kdbus_pool_slice *slice)
 	if (pool->slices.next != &slice->entry) {
 		struct kdbus_pool_slice *s;
 
-		s = list_entry(slice->entry.prev,
-			       struct kdbus_pool_slice, entry);
+		s = list_prev_entry(slice, entry);
 		if (s->free) {
 			/*
 			 * As size of slice increases after merge and free
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242245 — [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask()
Message-ID<qhhIf-2d4-55@gated-at.bofh.it>
In reply to#1242233
Replace the expression with more concise and readable equivalent. It can
be proven by opening parentheses:

    r & ~((p | i) & r) == r & (~(p | i) | ~r) ==
        (r & ~(p | i)) | (r & ~r) == r & ~(p | i) == r & ~p & ~i

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/metadata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index 788b4d9c7ecb..61215a078359 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -1321,7 +1321,7 @@ static u64 kdbus_meta_get_mask(struct pid *prv_pid, u64 prv_mask,
 	 * the sender, but still requested by the receiver. If any are left,
 	 * perform rather expensive /proc access checks for them.
 	 */
-	missing = req_mask & ~((prv_mask | impl_mask) & req_mask);
+	missing = req_mask & ~prv_mask & ~impl_mask;
 	if (missing)
 		proc_mask = kdbus_meta_proc_mask(prv_pid, req_pid, req_cred,
 						 missing);
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242407 — Re: [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask()

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-10-08 16:30 +0200
SubjectRe: [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask()
Message-ID<qhkmK-67a-15@gated-at.bofh.it>
In reply to#1242245
Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> Replace the expression with more concise and readable equivalent. It can
> be proven by opening parentheses:
>
>     r & ~((p | i) & r) == r & (~(p | i) | ~r) ==
>         (r & ~(p | i)) | (r & ~r) == r & ~(p | i) == r & ~p & ~i

But why? The current code follows the description, and does exactly
the same. It shows that it merges the "provided" and "implied" masks,
and then extracts the flags that are missing compared to the required
mask.

I cannot follow why your code is more obvious?
David

> Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> ---
>  ipc/kdbus/metadata.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
> index 788b4d9c7ecb..61215a078359 100644
> --- a/ipc/kdbus/metadata.c
> +++ b/ipc/kdbus/metadata.c
> @@ -1321,7 +1321,7 @@ static u64 kdbus_meta_get_mask(struct pid *prv_pid, u64 prv_mask,
>          * the sender, but still requested by the receiver. If any are left,
>          * perform rather expensive /proc access checks for them.
>          */
> -       missing = req_mask & ~((prv_mask | impl_mask) & req_mask);
> +       missing = req_mask & ~prv_mask & ~impl_mask;
>         if (missing)
>                 proc_mask = kdbus_meta_proc_mask(prv_pid, req_pid, req_cred,
>                                                  missing);
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1243573 — Re: [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-09 20:00 +0200
SubjectRe: [PATCH 15/44] kdbus: Simplify bitwise expression in kdbus_meta_get_mask()
Message-ID<qhK7x-Tp-43@gated-at.bofh.it>
In reply to#1242407
Hi David,

On Thu, Oct 08, 2015 at 04:24:30PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@s15v.net> wrote:
> > Replace the expression with more concise and readable equivalent. It can
> > be proven by opening parentheses:
> >
> >     r & ~((p | i) & r) == r & (~(p | i) | ~r) ==
> >         (r & ~(p | i)) | (r & ~r) == r & ~(p | i) == r & ~p & ~i
> 
> But why? The current code follows the description, and does exactly
> the same. It shows that it merges the "provided" and "implied" masks,
> and then extracts the flags that are missing compared to the required
> mask.
> 
> I cannot follow why your code is more obvious?

The variant I propose has one to one correspondence to the description,
but is shorter and has no multi levels of parentheses, thus easier to
read, IMO. The comment says "... set of metadata that is not granted
implicitly" (which is ~impl_mask), "... nor by the sender" (~prv_mask),
"... but still requested by the receiver" (req_mask).

We can leave parentheses, i.e. 'req_mask & ~(prv_mask | impl_mask)', but
even in this case the original code does one extra bitwise AND.

> David
> 
> > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
> > ---
> >  ipc/kdbus/metadata.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
> > index 788b4d9c7ecb..61215a078359 100644
> > --- a/ipc/kdbus/metadata.c
> > +++ b/ipc/kdbus/metadata.c
> > @@ -1321,7 +1321,7 @@ static u64 kdbus_meta_get_mask(struct pid *prv_pid, u64 prv_mask,
> >          * the sender, but still requested by the receiver. If any are left,
> >          * perform rather expensive /proc access checks for them.
> >          */
> > -       missing = req_mask & ~((prv_mask | impl_mask) & req_mask);
> > +       missing = req_mask & ~prv_mask & ~impl_mask;
> >         if (missing)
> >                 proc_mask = kdbus_meta_proc_mask(prv_pid, req_pid, req_cred,
> >                                                  missing);
> > --
> > 1.8.3.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1242247 — [PATCH 08/44] kdbus: Rename var in kdbus_meta_export_caps()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 08/44] kdbus: Rename var in kdbus_meta_export_caps()
Message-ID<qhhIf-2d4-51@gated-at.bofh.it>
In reply to#1242233
Rename `parent' to `member'. We currently set `parent' to true during
iteration over namespaces if `cred' resides in one of parent namespaces
of `user_ns'. But if `cred' is a member of `user_ns' itself, `parent'
name is not the best choice. Also new `member' name is more consistent
with the `owner' var, as it can be read "cred is a member of user_ns or
one of its parents" and "cred is the owner of ...".

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/metadata.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index 4ff4b99a40e0..788b4d9c7ecb 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -725,7 +725,7 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
 {
 	struct user_namespace *iter;
 	const struct cred *cred = mp->cred;
-	bool parent = false, owner = false;
+	bool member = false, owner = false;
 	int i;
 
 	/*
@@ -748,7 +748,7 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
 	 */
 	for (iter = user_ns; iter; iter = iter->parent) {
 		if (iter == cred->user_ns) {
-			parent = true;
+			member = true;
 			break;
 		}
 
@@ -765,7 +765,7 @@ static void kdbus_meta_export_caps(struct kdbus_meta_caps *out,
 	out->last_cap = CAP_LAST_CAP;
 
 	CAP_FOR_EACH_U32(i) {
-		if (parent) {
+		if (member) {
 			out->set[0].caps[i] = cred->cap_inheritable.cap[i];
 			out->set[1].caps[i] = cred->cap_permitted.cap[i];
 			out->set[2].caps[i] = cred->cap_effective.cap[i];
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242248 — [PATCH 41/44] kdbus: Fix memfd install algorithm

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 41/44] kdbus: Fix memfd install algorithm
Message-ID<qhhIf-2d4-63@gated-at.bofh.it>
In reply to#1242233
If file descriptor allocation for memfd fails, we do not fill the
corresponding position in `fds' array with -1. Later when we install
memfds, fds[gaps->n_fds + i] will contain garbage which we pass then
to fd_install(). Fix it by adding -1 to `fds' in case when we can't
get free file descriptor for memfd.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/message.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index f2176796390d..0653a085c104 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -181,6 +181,7 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 		memfd = get_unused_fd_flags(O_CLOEXEC);
 		if (memfd < 0) {
 			incomplete_fds = true;
+			fds[n_fds++] = -1;
 			/* memfds are initialized to -1, skip copying it */
 			continue;
 		}
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242249 — [PATCH 27/44] kdbus: Cleanup kdbus_conn_new()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 27/44] kdbus: Cleanup kdbus_conn_new()
Message-ID<qhhIf-2d4-57@gated-at.bofh.it>
In reply to#1242233
 - Replace two tests with one. Sequence of tests

       name && !is_activator && !is_policy_holder
       !name && (is_activator || is_policy_holder)

   is the same as

       name XOR (is_activator || is_policy_holder)

   Replace these two expressions with

       !name == (is_activator || is_policy_holder)

 - Drop `privileged' var which is used only once to set value of
   conn->privileged.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index ace587ee951a..c57ff2c846ee 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -73,7 +73,6 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
 	bool is_policy_holder;
 	bool is_activator;
 	bool is_monitor;
-	bool privileged;
 	bool owner;
 	struct kvec kvec;
 	int ret;
@@ -84,9 +83,7 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
 		struct kdbus_bloom_parameter bloom;
 	} bloom_item;
 
-	privileged = kdbus_ep_is_privileged(ep, file);
 	owner = kdbus_ep_is_owner(ep, file);
-
 	is_monitor = hello->flags & KDBUS_HELLO_MONITOR;
 	is_activator = hello->flags & KDBUS_HELLO_ACTIVATOR;
 	is_policy_holder = hello->flags & KDBUS_HELLO_POLICY_HOLDER;
@@ -95,9 +92,7 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
 		return ERR_PTR(-EINVAL);
 	if (is_monitor + is_activator + is_policy_holder > 1)
 		return ERR_PTR(-EINVAL);
-	if (name && !is_activator && !is_policy_holder)
-		return ERR_PTR(-EINVAL);
-	if (!name && (is_activator || is_policy_holder))
+	if (!name == (is_activator || is_policy_holder))
 		return ERR_PTR(-EINVAL);
 	if (name && !kdbus_name_is_valid(name, true))
 		return ERR_PTR(-EINVAL);
@@ -138,7 +133,7 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
 	get_fs_root(current->fs, &conn->root_path);
 	init_waitqueue_head(&conn->wait);
 	kdbus_queue_init(&conn->queue);
-	conn->privileged = privileged;
+	conn->privileged = kdbus_ep_is_privileged(ep, file);
 	conn->owner = owner;
 	conn->ep = kdbus_ep_ref(ep);
 	conn->id = atomic64_inc_return(&bus->domain->last_id);
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1242250 — [PATCH 20/44] kdbus: Drop useless initialization from kdbus_cmd_hello()

FromSergei Zviagintsev <sergei@s15v.net>
Date2015-10-08 13:40 +0200
Subject[PATCH 20/44] kdbus: Drop useless initialization from kdbus_cmd_hello()
Message-ID<qhhIf-2d4-61@gated-at.bofh.it>
In reply to#1242233
Do not initialize `c' as it is assigned a return value of
kdbus_conn_new() at the first use.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 ipc/kdbus/connection.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 4b5ed4bb59c7..93da7f539f74 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -1591,7 +1591,7 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, struct file *file,
 				   void __user *argp)
 {
 	struct kdbus_cmd_hello *cmd;
-	struct kdbus_conn *c = NULL;
+	struct kdbus_conn *c;
 	const char *item_name;
 	int ret;
 
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


Page 1 of 4  [1] 2 3 4  Next page →

Back to top | Article view | linux.kernel


csiph-web