All of lore.kernel.org
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: akpm@linux-foundation.org
Cc: manfred@colorfullife.com, dave@stgolabs.net,
	linux-kernel@vger.kernel.org, Davidlohr Bueso <dbueso@suse.de>
Subject: [PATCH 4/4] sysvipc: make get_maxid O(1) again
Date: Thu, 31 Aug 2017 10:20:49 -0700	[thread overview]
Message-ID: <20170831172049.14576-5-dave@stgolabs.net> (raw)
In-Reply-To: <20170831172049.14576-1-dave@stgolabs.net>

For a custom microbenchmark on a 3.30GHz Xeon SandyBridge,
which calls IPC_STAT over and over, it was calculated that,
on avg the cost of ipc_get_maxid() for increasing amounts of
keys was:

10 keys: ~900 cycles
100 keys: ~15000 cycles
1000 keys: ~150000 cycles
10000 keys: ~2100000 cycles

This is unsurprising as maxid is currently O(n).

By having the max_id available in O(1) we save all those cycles
for each semctl(_STAT) command, the idr_find can be expensive --
which some real (customer) workloads actually poll on. Note that
this used to be the case, until 7ca7e564e04 (ipc: store ipcs into
IDRs). The cost is the extra idr_find when doing RMIDs, but we
simply go backwards, and should not take too many iterations to
find the new value.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 arch/x86/Kconfig              |  2 +-
 include/linux/ipc_namespace.h |  1 +
 ipc/util.c                    | 43 +++++++++++++------------------------------
 ipc/util.h                    | 21 ++++++++++++++++++---
 4 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 44d139b88c32..301fe235367b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -55,7 +55,7 @@ config X86
 	select ARCH_HAS_KCOV			if X86_64
 	select ARCH_HAS_MMIO_FLUSH
 	select ARCH_HAS_PMEM_API		if X86_64
-	select ARCH_HAS_REFCOUNT
+	select ARCH_HAS_REFCOUNT		if BROKEN
 	select ARCH_HAS_UACCESS_FLUSHCACHE	if X86_64
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_SG_CHAIN
diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 058051566ced..c59b8ddee191 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -18,6 +18,7 @@ struct ipc_ids {
 	bool tables_initialized;
 	struct rw_semaphore rwsem;
 	struct idr ipcs_idr;
+	int max_id;
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	int next_id;
 #endif
diff --git a/ipc/util.c b/ipc/util.c
index d4091665f439..45e84203c3fe 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -121,6 +121,7 @@ int ipc_init_ids(struct ipc_ids *ids)
 		return err;
 	idr_init(&ids->ipcs_idr);
 	ids->tables_initialized = true;
+	ids->max_id = -1;
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	ids->next_id = -1;
 #endif
@@ -187,36 +188,6 @@ static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
 	return NULL;
 }
 
-/**
- * ipc_get_maxid - get the last assigned id
- * @ids: ipc identifier set
- *
- * Called with ipc_ids.rwsem held.
- */
-int ipc_get_maxid(struct ipc_ids *ids)
-{
-	struct kern_ipc_perm *ipc;
-	int max_id = -1;
-	int total, id;
-
-	if (ids->in_use == 0)
-		return -1;
-
-	if (ids->in_use == IPCMNI)
-		return IPCMNI - 1;
-
-	/* Look for the last assigned id */
-	total = 0;
-	for (id = 0; id < IPCMNI && total < ids->in_use; id++) {
-		ipc = idr_find(&ids->ipcs_idr, id);
-		if (ipc != NULL) {
-			max_id = id;
-			total++;
-		}
-	}
-	return max_id;
-}
-
 #ifdef CONFIG_CHECKPOINT_RESTORE
 /*
  * Specify desired id for next allocated IPC object.
@@ -312,6 +283,9 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
 	}
 
 	ids->in_use++;
+	if (id > ids->max_id)
+		ids->max_id = id;
+
 	new->id = ipc_buildid(id, ids, new);
 
 	return id;
@@ -458,6 +432,15 @@ void ipc_rmid(struct ipc_ids *ids, struct kern_ipc_perm *ipcp)
 	ipc_kht_remove(ids, ipcp);
 	ids->in_use--;
 	ipcp->deleted = true;
+
+	if (unlikely(lid == ids->max_id)) {
+		do {
+			lid--;
+			if(lid == -1)
+				break;
+		} while (!idr_find(&ids->ipcs_idr, lid));
+		ids->max_id = lid;
+	}
 }
 
 /**
diff --git a/ipc/util.h b/ipc/util.h
index 17c8d2f14990..eabca0e37ce1 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -12,6 +12,7 @@
 
 #include <linux/unistd.h>
 #include <linux/err.h>
+#include <linux/ipc_namespace.h>
 
 #define SEQ_MULTIPLIER	(IPCMNI)
 
@@ -98,9 +99,6 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
 /* must be called with ids->rwsem acquired for writing */
 int ipc_addid(struct ipc_ids *, struct kern_ipc_perm *, int);
 
-/* must be called with ids->rwsem acquired for reading */
-int ipc_get_maxid(struct ipc_ids *);
-
 /* must be called with both locks acquired. */
 void ipc_rmid(struct ipc_ids *, struct kern_ipc_perm *);
 
@@ -110,6 +108,23 @@ void ipc_set_key_private(struct ipc_ids *, struct kern_ipc_perm *);
 /* must be called with ipcp locked */
 int ipcperms(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp, short flg);
 
+/**
+ * ipc_get_maxid - get the last assigned id
+ * @ids: ipc identifier set
+ *
+ * Called with ipc_ids.rwsem held for reading.
+ */
+static inline int ipc_get_maxid(struct ipc_ids *ids)
+{
+	if (ids->in_use == 0)
+		return -1;
+
+	if (ids->in_use == IPCMNI)
+		return IPCMNI - 1;
+
+	return ids->max_id;
+}
+
 /*
  * For allocation that need to be freed by RCU.
  * Objects are reference counted, they start with reference count 1.
-- 
2.12.0

  parent reply	other threads:[~2017-08-31 17:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-31 17:20 [PATCH -next 0/4] sysvipc: ipc-key management improvements Davidlohr Bueso
2017-08-31 17:20 ` [PATCH 1/4] sysvipc: unteach ids->next_id for !CHECKPOINT_RESTORE Davidlohr Bueso
2017-10-03 22:03   ` Andrew Morton
2017-08-31 17:20 ` [PATCH 2/4] sysvipc: duplicate lock comments wrt ipc_addid() Davidlohr Bueso
2017-08-31 17:20 ` [PATCH 3/4] sysvipc: properly name ipc_addid() limit parameter Davidlohr Bueso
2017-08-31 17:20 ` Davidlohr Bueso [this message]
2017-08-31 17:37   ` [PATCH 4/4] sysvipc: make get_maxid O(1) again Davidlohr Bueso

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=20170831172049.14576-5-dave@stgolabs.net \
    --to=dave@stgolabs.net \
    --cc=akpm@linux-foundation.org \
    --cc=dbueso@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manfred@colorfullife.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.