Module Name: src
Committed By: riastradh
Date: Mon Aug 27 14:14:42 UTC 2018
Modified Files:
src/sys/external/bsd/drm2/include/linux: idr.h
src/sys/external/bsd/drm2/linux: linux_idr.c
Log Message:
Draft rewrite of idr preload.
Previous idr code assumed every caller would definitely call
idr_preload, idr_alloc, idr_preload_end, but some callers skip
idr_alloc if an intermediate failure happens first, and would
therefore leak idr nodes.
Use a per-lwp single-node cache instead, and print warnings about
leakers.
To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/sys/external/bsd/drm2/include/linux/idr.h
cvs rdiff -u -r1.7 -r1.8 src/sys/external/bsd/drm2/linux/linux_idr.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/external/bsd/drm2/include/linux/idr.h
diff -u src/sys/external/bsd/drm2/include/linux/idr.h:1.6 src/sys/external/bsd/drm2/include/linux/idr.h:1.7
--- src/sys/external/bsd/drm2/include/linux/idr.h:1.6 Mon Aug 27 07:15:06 2018
+++ src/sys/external/bsd/drm2/include/linux/idr.h Mon Aug 27 14:14:42 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: idr.h,v 1.6 2018/08/27 07:15:06 riastradh Exp $ */
+/* $NetBSD: idr.h,v 1.7 2018/08/27 14:14:42 riastradh Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -33,9 +33,7 @@
#define _LINUX_IDR_H_
#include <sys/types.h>
-#include <sys/rwlock.h>
#include <sys/rbtree.h>
-#include <sys/queue.h>
#include <linux/gfp.h>
Index: src/sys/external/bsd/drm2/linux/linux_idr.c
diff -u src/sys/external/bsd/drm2/linux/linux_idr.c:1.7 src/sys/external/bsd/drm2/linux/linux_idr.c:1.8
--- src/sys/external/bsd/drm2/linux/linux_idr.c:1.7 Mon Aug 27 06:55:01 2018
+++ src/sys/external/bsd/drm2/linux/linux_idr.c Mon Aug 27 14:14:42 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: linux_idr.c,v 1.7 2018/08/27 06:55:01 riastradh Exp $ */
+/* $NetBSD: linux_idr.c,v 1.8 2018/08/27 14:14:42 riastradh Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: linux_idr.c,v 1.7 2018/08/27 06:55:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_idr.c,v 1.8 2018/08/27 14:14:42 riastradh Exp $");
#include <sys/param.h>
#include <sys/atomic.h>
@@ -40,27 +40,73 @@ __KERNEL_RCSID(0, "$NetBSD: linux_idr.c,
#include <linux/idr.h>
#include <linux/slab.h>
+#ifdef _KERNEL_OPT
+#include "opt_ddb.h"
+#endif
+
+#ifdef DDB
+#include <ddb/ddb.h>
+#endif
+
struct idr_node {
rb_node_t in_rb_node;
int in_index;
void *in_data;
- SIMPLEQ_ENTRY(idr_node) in_list;
};
-SIMPLEQ_HEAD(idr_head, idr_node);
-static struct {
- kmutex_t lock;
- struct idr_head preloaded_nodes;
- struct idr_head discarded_nodes;
-} idr_cache __cacheline_aligned;
+struct idr_cache {
+ struct idr_node *ic_node;
+ void *ic_where;
+};
+
+static specificdata_key_t idr_cache_key __read_mostly;
+
+static void
+idr_cache_warning(struct idr_cache *cache)
+{
+#ifdef DDB
+ const char *name;
+ db_expr_t offset;
+#endif
+
+ KASSERT(cache->ic_node != NULL);
+
+#ifdef DDB
+ db_find_sym_and_offset((db_addr_t)(uintptr_t)cache->ic_where,
+ &name, &offset);
+ if (name) {
+ printf("WARNING: idr preload at %s+%#"DDB_EXPR_FMT"x"
+ " leaked in lwp %s @ %p\n",
+ name, offset, curlwp->l_name, curlwp);
+ } else
+#endif
+ {
+ printf("WARNING: idr preload at %p leaked in lwp %s @ %p\n",
+ cache->ic_where, curlwp->l_name, curlwp);
+ }
+}
+
+static void
+idr_cache_dtor(void *cookie)
+{
+ struct idr_cache *cache = cookie;
+
+ if (cache->ic_node) {
+ idr_cache_warning(cache);
+ kmem_free(cache->ic_node, sizeof(*cache->ic_node));
+ }
+ kmem_free(cache, sizeof(*cache));
+}
int
linux_idr_module_init(void)
{
+ int error;
+
+ error = lwp_specific_key_create(&idr_cache_key, &idr_cache_dtor);
+ if (error)
+ return error;
- mutex_init(&idr_cache.lock, MUTEX_DEFAULT, IPL_VM);
- SIMPLEQ_INIT(&idr_cache.preloaded_nodes);
- SIMPLEQ_INIT(&idr_cache.discarded_nodes);
return 0;
}
@@ -68,9 +114,7 @@ void
linux_idr_module_fini(void)
{
- KASSERT(SIMPLEQ_EMPTY(&idr_cache.discarded_nodes));
- KASSERT(SIMPLEQ_EMPTY(&idr_cache.preloaded_nodes));
- mutex_destroy(&idr_cache.lock);
+ lwp_specific_key_delete(idr_cache_key);
}
static signed int idr_tree_compare_nodes(void *, const void *, const void *);
@@ -198,31 +242,65 @@ idr_remove(struct idr *idr, int id)
KASSERTMSG((node != NULL), "idr %p has no entry for id %d", idr, id);
rb_tree_remove_node(&idr->idr_tree, node);
mutex_spin_exit(&idr->idr_lock);
- kfree(node);
+
+ kmem_free(node, sizeof(*node));
}
void
idr_preload(gfp_t gfp)
{
+ struct idr_cache *cache;
struct idr_node *node;
+ km_flag_t kmflag = ISSET(gfp, __GFP_WAIT) ? KM_SLEEP : KM_NOSLEEP;
+ /* If caller asked to wait, we had better be sleepable. */
if (ISSET(gfp, __GFP_WAIT))
ASSERT_SLEEPABLE();
- node = kmalloc(sizeof(*node), gfp);
+ /*
+ * Get the current lwp's private idr cache.
+ */
+ cache = lwp_getspecific(idr_cache_key);
+ if (cache == NULL) {
+ /* lwp_setspecific must be sleepable. */
+ if (!ISSET(gfp, __GFP_WAIT))
+ return;
+ cache = kmem_alloc(sizeof(*cache), kmflag);
+ if (cache == NULL)
+ return;
+ lwp_setspecific(idr_cache_key, cache);
+ }
+
+ /*
+ * If there already is a node, a prior call to idr_preload must
+ * not have been matched by idr_preload_end. Print a warning,
+ * claim the node, and record our return address for where this
+ * node came from so the next leak is attributed to us.
+ */
+ if (cache->ic_node) {
+ idr_cache_warning(cache);
+ goto out;
+ }
+
+ /*
+ * No cached node. Allocate a new one, store it in the cache,
+ * and record our return address for where this node came from
+ * so the next leak is attributed to us.
+ */
+ node = kmem_alloc(sizeof(*node), kmflag);
KASSERT(node != NULL || !ISSET(gfp, __GFP_WAIT));
if (node == NULL)
return;
- mutex_spin_enter(&idr_cache.lock);
- SIMPLEQ_INSERT_TAIL(&idr_cache.preloaded_nodes, node, in_list);
- mutex_spin_exit(&idr_cache.lock);
+ cache->ic_node = node;
+out: cache->ic_where = __builtin_return_address(0);
}
int
idr_alloc(struct idr *idr, void *data, int start, int end, gfp_t gfp)
{
int maximum = (end <= 0? INT_MAX : (end - 1));
+ struct idr_cache *cache;
struct idr_node *node, *search, *collision __diagused;
int id = start;
@@ -234,15 +312,15 @@ idr_alloc(struct idr *idr, void *data, i
if (__predict_false(maximum < start))
return -ENOSPC;
- /* Grab a node allocated by idr_preload. */
- mutex_spin_enter(&idr_cache.lock);
- if (SIMPLEQ_EMPTY(&idr_cache.preloaded_nodes)) {
- mutex_spin_exit(&idr_cache.lock);
+ /*
+ * Grab a node allocated by idr_preload, if we have a cache and
+ * it is populated.
+ */
+ cache = lwp_getspecific(idr_cache_key);
+ if (cache == NULL || cache->ic_node == NULL)
return -ENOMEM;
- }
- node = SIMPLEQ_FIRST(&idr_cache.preloaded_nodes);
- SIMPLEQ_REMOVE_HEAD(&idr_cache.preloaded_nodes, in_list);
- mutex_spin_exit(&idr_cache.lock);
+ node = cache->ic_node;
+ cache->ic_node = NULL;
/* Find an id. */
mutex_spin_enter(&idr->idr_lock);
@@ -261,31 +339,38 @@ idr_alloc(struct idr *idr, void *data, i
KASSERT(collision == node);
out: mutex_spin_exit(&idr->idr_lock);
- if (id < 0) {
- /* Discard the node on failure. */
- mutex_spin_enter(&idr_cache.lock);
- SIMPLEQ_INSERT_HEAD(&idr_cache.discarded_nodes, node, in_list);
- mutex_spin_exit(&idr_cache.lock);
- }
+ /* Discard the node on failure. */
+ if (id < 0)
+ cache->ic_node = node;
return id;
}
void
idr_preload_end(void)
{
- struct idr_head temp = SIMPLEQ_HEAD_INITIALIZER(temp);
- struct idr_node *node, *next;
+ struct idr_cache *cache;
- mutex_spin_enter(&idr_cache.lock);
- SIMPLEQ_FOREACH_SAFE(node, &idr_cache.discarded_nodes, in_list, next) {
- SIMPLEQ_REMOVE_HEAD(&idr_cache.discarded_nodes, in_list);
- SIMPLEQ_INSERT_HEAD(&temp, node, in_list);
- }
- mutex_spin_exit(&idr_cache.lock);
+ /* Get the cache, or bail if it's not there. */
+ cache = lwp_getspecific(idr_cache_key);
+ if (cache == NULL)
+ return;
+
+ /*
+ * If there is a node, either because we didn't idr_alloc or
+ * because idr_alloc failed, chuck it.
+ *
+ * XXX If we are not sleepable, then while the caller may have
+ * used idr_preload(GFP_ATOMIC), kmem_free may still sleep.
+ * What to do?
+ */
+ if (cache->ic_node) {
+ struct idr_node *node;
+
+ node = cache->ic_node;
+ cache->ic_node = NULL;
+ cache->ic_where = NULL;
- SIMPLEQ_FOREACH_SAFE(node, &temp, in_list, next) {
- SIMPLEQ_REMOVE_HEAD(&temp, in_list);
- kfree(node);
+ kmem_free(node, sizeof(*node));
}
}
