[DTrace-devel] [PATCH v3 07/11] htab reduction: kernpath

Nick Alcock nick.alcock at oracle.com
Mon Nov 1 14:29:43 UTC 2021


Replace the special-purpose kernpath hash table with the generic
dt_htab.  This is a simple case which serves as a template for the
more complex ones to follow.

This has very little effect on performance, because even though the htab
is terribly poorly sized, it is sized in the wrong direction: the
kernpath hash only contains out-of-tree modules, and there won't be
hundreds of those.  The main improvement here is in clarity.

Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
 libdtrace/dt_impl.h          |   7 +--
 libdtrace/dt_kernel_module.c | 109 +++++++++++++++++++----------------
 libdtrace/dt_kernel_module.h |   3 +-
 libdtrace/dt_open.c          |  15 ++---
 4 files changed, 66 insertions(+), 68 deletions(-)

diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index 839660d99c17..e4611908d12c 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -159,7 +159,7 @@ typedef struct dt_module {
  */
 typedef struct dt_kern_path {
 	dt_list_t dkp_list;	       /* list forward/back pointers */
-	struct dt_kern_path *dkp_next; /* hash chain next pointer */
+	struct dt_hentry dkp_he;       /* htab links */
 	char *dkp_name;		       /* name of this kernel module */
 	char *dkp_path;		       /* full name including path */
 } dt_kern_path_t;
@@ -300,10 +300,7 @@ struct dtrace_hdl {
 	ctf_archive_t *dt_ctfa; /* ctf archive for the entire kernel tree */
 	ctf_file_t *dt_shared_ctf; /* Handle to the shared CTF */
 	char *dt_ctfa_path;	/* path to vmlinux.ctfa */
-	dt_list_t dt_kernpathlist; /* linked list of dt_kern_path_t's */
-	dt_kern_path_t **dt_kernpaths; /* hash table of dt_kern_path_t's */
-	uint_t dt_kernpathbuckets; /* number of kernel module path hash buckets */
-	uint_t dt_nkernpaths;	/* number of kernel module paths in hash and list */
+	dt_htab_t *dt_kernpaths; /* hash table of dt_kern_path_t's */
 	dt_module_t *dt_exec;	/* pointer to executable module */
 	dt_module_t *dt_cdefs;	/* pointer to C dynamic type module */
 	dt_module_t *dt_ddefs;	/* pointer to D dynamic type module */
diff --git a/libdtrace/dt_kernel_module.c b/libdtrace/dt_kernel_module.c
index 47a4cc04e0bf..1d147405af9d 100644
--- a/libdtrace/dt_kernel_module.c
+++ b/libdtrace/dt_kernel_module.c
@@ -29,66 +29,78 @@
 
 static pthread_mutex_t kern_path_update_lock = PTHREAD_MUTEX_INITIALIZER;
 
+static uint32_t
+kernpath_hval(const dt_kern_path_t *dkp)
+{
+	return str2hval(dkp->dkp_name, 0);
+}
+
+static int
+kernpath_cmp(const dt_kern_path_t *p,
+	     const dt_kern_path_t *q)
+{
+	return strcmp(p->dkp_name, q->dkp_name);
+}
+
+DEFINE_HE_STD_LINK_FUNCS(kernpath, dt_kern_path_t, dkp_he)
+
+static void *
+kernpath_del_path(dt_kern_path_t *head, dt_kern_path_t *dkpp)
+{
+	head = kernpath_del(head, dkpp);
+	free(dkpp->dkp_name);
+	free(dkpp->dkp_path);
+	free(dkpp);
+	return head;
+}
+
+static dt_htab_ops_t kernpath_htab_ops = {
+	.hval = (htab_hval_fn)kernpath_hval,
+	.cmp = (htab_cmp_fn)kernpath_cmp,
+	.add = (htab_add_fn)kernpath_add,
+	.del = (htab_del_fn)kernpath_del_path
+};
+
 /*
  * On successful return, name and path become owned by dt_kern_path_create()
- * (and may be freed immediately).
- *
- * Note: this code is used by ctf_module_dump as well.
+ * (and may be freed immediately).  The returned entry is owned by the
+ * dt_kernpaths hash into which it is interned.
  */
 
 dt_kern_path_t *
 dt_kern_path_create(dtrace_hdl_t *dtp, char *name, char *path)
 {
-	uint_t h = str2hval(name, 0) % dtp->dt_kernpathbuckets;
 	dt_kern_path_t *dkpp;
+	dt_kern_path_t tmpl;
 
-	for (dkpp = dtp->dt_kernpaths[h]; dkpp != NULL; dkpp = dkpp->dkp_next) {
-		if (strcmp(dkpp->dkp_name, name) == 0) {
-			free(name);
-			free(path);
-			return dkpp;
-		}
+	if (!dtp->dt_kernpaths) {
+		dtp->dt_kernpaths = dt_htab_create(dtp, &kernpath_htab_ops);
+
+		if (!dtp->dt_kernpaths)
+			return NULL; /* caller must handle allocation failure */
+	}
+
+	tmpl.dkp_name = name;
+	if ((dkpp = dt_htab_lookup(dtp->dt_kernpaths, &tmpl)) != NULL) {
+		free(name);
+		free(path);
+		return dkpp;
 	}
 
 	if ((dkpp = malloc(sizeof(dt_kern_path_t))) == NULL)
-		return NULL; /* caller must handle allocation failure */
+		return NULL;
 
 	dt_dprintf("Adding %s -> %s\n", name, path);
 
 	memset(dkpp, 0, sizeof(dt_kern_path_t));
 	dkpp->dkp_name = name;
 	dkpp->dkp_path = path;			/* strdup()ped by our caller */
-	dt_list_append(&dtp->dt_kernpathlist, dkpp);
-	dkpp->dkp_next = dtp->dt_kernpaths[h];
-	dtp->dt_kernpaths[h] = dkpp;
-	dtp->dt_nkernpaths++;
-
-	return dkpp;
-}
-
-void
-dt_kern_path_destroy(dtrace_hdl_t *dtp, dt_kern_path_t *dkpp)
-{
-	uint_t h = str2hval(dkpp->dkp_name, 0) % dtp->dt_kernpathbuckets;
-	dt_kern_path_t *scan_dkpp;
-	dt_kern_path_t *prev_dkpp = NULL;
-
-	dt_list_delete(&dtp->dt_kernpathlist, dkpp);
-	assert(dtp->dt_nkernpaths != 0);
-	dtp->dt_nkernpaths--;
-
-	for (scan_dkpp = dtp->dt_kernpaths[h]; (scan_dkpp != NULL) &&
-		 (scan_dkpp != dkpp); scan_dkpp = scan_dkpp->dkp_next) {
-		prev_dkpp = scan_dkpp;
+	if (dt_htab_insert(dtp->dt_kernpaths, dkpp) < 0) {
+		free(dkpp);
+		return NULL;
 	}
-	if (prev_dkpp == NULL)
-		dtp->dt_kernpaths[h] = dkpp->dkp_next;
-	else
-		prev_dkpp->dkp_next = dkpp->dkp_next;
 
-	free(dkpp->dkp_name);
-	free(dkpp->dkp_path);
-	free(dkpp);
+	return dkpp;
 }
 
 /*
@@ -185,10 +197,9 @@ dt_kern_path_update(dtrace_hdl_t *dtp)
 dt_kern_path_t *
 dt_kern_path_lookup_by_name(dtrace_hdl_t *dtp, const char *name)
 {
-	uint_t h = str2hval(name, 0) % dtp->dt_kernpathbuckets;
-	dt_kern_path_t *dkpp;
+	dt_kern_path_t tmpl;
 
-	if (dtp->dt_nkernpaths == 0) {
+	if (!dtp->dt_kernpaths) {
 		int dterrno;
 
 		pthread_mutex_lock(&kern_path_update_lock);
@@ -201,14 +212,10 @@ dt_kern_path_lookup_by_name(dtrace_hdl_t *dtp, const char *name)
 			return NULL;
 		}
 
-		dt_dprintf("Initialized %i kernel module paths\n",
-		    dtp->dt_nkernpaths);
-	}
-
-	for (dkpp = dtp->dt_kernpaths[h]; dkpp != NULL; dkpp = dkpp->dkp_next) {
-		if (strcmp(dkpp->dkp_name, name) == 0)
-			return dkpp;
+		dt_dprintf("Initialized %zi kernel module paths\n",
+		    dt_htab_entries(dtp->dt_kernpaths));
 	}
 
-	return NULL;
+	tmpl.dkp_name = (char *) name;
+	return dt_htab_lookup(dtp->dt_kernpaths, &tmpl);
 }
diff --git a/libdtrace/dt_kernel_module.h b/libdtrace/dt_kernel_module.h
index 3397106a708a..902457d722fc 100644
--- a/libdtrace/dt_kernel_module.h
+++ b/libdtrace/dt_kernel_module.h
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -15,6 +15,5 @@ extern dt_kern_path_t *dt_kern_path_create(dtrace_hdl_t *dtp, char *name,
 extern int dt_kern_path_update(dtrace_hdl_t *dtp);
 extern dt_kern_path_t *dt_kern_path_lookup_by_name(dtrace_hdl_t *dtp,
     const char *name);
-extern void dt_kern_path_destroy(dtrace_hdl_t *dtp, dt_kern_path_t *dkpp);
 
 #endif	/* _DT_KERNEL_MODULE_H */
diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index 25b3e5b8cf70..1681940f1f74 100644
--- a/libdtrace/dt_open.c
+++ b/libdtrace/dt_open.c
@@ -736,8 +736,6 @@ dt_vopen(int version, int flags, int *errp,
 	dtp->dt_poll_fd = -1;
 	dtp->dt_modbuckets = _dtrace_strbuckets;
 	dtp->dt_mods = calloc(dtp->dt_modbuckets, sizeof(dt_module_t *));
-	dtp->dt_kernpathbuckets = _dtrace_strbuckets;
-	dtp->dt_kernpaths = calloc(dtp->dt_kernpathbuckets, sizeof(dt_kern_path_t *));
 	dtp->dt_provbuckets = _dtrace_strbuckets;
 	dtp->dt_provs = calloc(dtp->dt_provbuckets, sizeof(dt_provider_t *));
 	dt_proc_hash_create(dtp);
@@ -772,10 +770,10 @@ dt_vopen(int version, int flags, int *errp,
 	if (dt_str2kver(dtp->dt_uts.release, &dtp->dt_kernver) < 0)
 		return set_open_errno(dtp, errp, EDT_VERSINVAL);
 
-	if (dtp->dt_mods == NULL || dtp->dt_kernpaths == NULL || 
-	    dtp->dt_provs == NULL || dtp->dt_procs == NULL ||
-	    dtp->dt_ld_path == NULL || dtp->dt_cpp_path == NULL ||
-	    dtp->dt_cpp_argv == NULL || dtp->dt_sysslice == NULL)
+	if (dtp->dt_mods == NULL || dtp->dt_provs == NULL ||
+	    dtp->dt_procs == NULL || dtp->dt_ld_path == NULL ||
+	    dtp->dt_cpp_path == NULL || dtp->dt_cpp_argv == NULL ||
+	    dtp->dt_sysslice == NULL)
 		return set_open_errno(dtp, errp, EDT_NOMEM);
 
 	for (i = 0; i < DTRACEOPT_MAX; i++)
@@ -1185,7 +1183,6 @@ dtrace_close(dtrace_hdl_t *dtp)
 {
 	dt_ident_t *idp, *ndp;
 	dt_module_t *dmp;
-	dt_kern_path_t *dkpp;
 	dt_provider_t *pvp;
 	dtrace_prog_t *pgp;
 	dt_xlator_t *dxp;
@@ -1233,8 +1230,7 @@ dtrace_close(dtrace_hdl_t *dtp)
 	while ((dmp = dt_list_next(&dtp->dt_modlist)) != NULL)
 		dt_module_destroy(dtp, dmp);
 
-	while ((dkpp = dt_list_next(&dtp->dt_kernpathlist)) != NULL)
-		dt_kern_path_destroy(dtp, dkpp);
+	dt_htab_destroy(dtp, dtp->dt_kernpaths);
 
 	if (dtp->dt_shared_ctf != NULL)
 		ctf_close(dtp->dt_shared_ctf);
@@ -1286,7 +1282,6 @@ dtrace_close(dtrace_hdl_t *dtp)
 
 	free(dtp->dt_mods);
 	free(dtp->dt_module_path);
-	free(dtp->dt_kernpaths);
 	free(dtp->dt_provs);
 	free(dtp->dt_strtab);
 	free(dtp);
-- 
2.33.1.257.g9e0974a4e8




More information about the DTrace-devel mailing list