Subject: Re: Enhancing sysctl support in ld.elf_so
To: None <tech-userlevel@netbsd.org>
From: Quentin Garnier <netbsd@quatriemek.com>
List: tech-userlevel
Date: 06/29/2004 01:03:17
This is a multi-part message in MIME format.

--Multipart=_Tue__29_Jun_2004_01_03_17_+0200_Wp3mnT6sjBqFtcE+
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

New patch, new values.  Much better!

Same no-op test executable, linked against just libc, or libm, or libm and
libm387 (test, testm and testmm respectively).  Report to my previous post
for more details on the test.

[Standard ld.so.conf, new ld.elf_so]
test   -> 18.89s user 22.27s system 104% cpu 39.342 total
testm  -> 24.33s user 28.90s system 103% cpu 51.376 total
testmm -> 24.02s user 29.11s system 103% cpu 51.257 total

[Empty ld.so.conf, new ld.elf_so]
test   -> 18.45s user 21.25s system 104% cpu 37.874 total
testm  -> 21.17s user 25.09s system 104% cpu 44.367 total
testmm -> 22.83s user 27.33s system 103% cpu 48.408 total

[Empty ld.so.conf, old ld.elf_so]
test   -> 18.53s user 21.54s system 104% cpu 38.318 total
testm  -> 21.54s user 25.14s system 104% cpu 44.829 total
testmm -> 23.32s user 27.56s system 103% cpu 49.094 total

[Standard ld.so.conf, old ld.elf_so]
test   -> 19.20s user 22.73s system 104% cpu 40.163 total
testm  -> 24.35s user 29.08s system 103% cpu 51.583 total
testmm -> 24.26s user 29.08s system 103% cpu 51.500 total

% size ld.elf_so.*
   text    data     bss     dec     hex filename
  42203    2840    2564   47607    b9f7 ld.elf_so.new
  42554    3228    2564   48346    bcda ld.elf_so.old

So, not only it is now smaller, but it's faster.  And there is obvious
room for optimisation.  I'm still having a hard time believing those data,
so if anyone would like to run a few tests...

I'm not surprised that 'test' with the new ld.elf_so and std ld.so.conf
runs faster, since it doesn't parse the sysctl name each time, only when
it needs it, but I expected in return the repeted calls to sysctl() to get
slower, even if not by much...

Comments?

-- 
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"Feels like I'm fiddling while Rome is burning down.
Should I lay my fiddle down and take a rifle from the ground ?"
Leigh Nash/Sixpence None The Richer, Paralyzed, Divine Discontents, 2002.

--Multipart=_Tue__29_Jun_2004_01_03_17_+0200_Wp3mnT6sjBqFtcE+
Content-Type: text/plain;
 name="diff"
Content-Disposition: attachment;
 filename="diff"
Content-Transfer-Encoding: 7bit

Index: load.c
===================================================================
RCS file: /cvsroot/src/libexec/ld.elf_so/load.c,v
retrieving revision 1.27
diff -u -r1.27 load.c
--- load.c	25 Nov 2003 14:36:49 -0000	1.27
+++ load.c	28 Jun 2004 22:27:33 -0000
@@ -170,6 +170,77 @@
 	return obj;
 }
 
+static int
+_rtld_sysctl(char *name, void *oldp, size_t *oldlen)
+{
+	char *node, *path;
+	struct sysctlnode query, *result, *newresult;
+	int mib[CTL_MAXNAME], i, r;
+	size_t res_size, n;
+	u_int miblen = 0;
+
+	/* Start with 16 entries, will grow it up as needed. */
+	res_size = 16 * sizeof(struct sysctlnode);
+	result = (struct sysctlnode *)malloc(res_size);
+	if (result == NULL)
+		return (-1);
+
+	/* We need a copy since we're modifying the string
+	 * XXX should use the functions in paths.c to avoid
+	 *     the copy */
+	path = strdup(name);
+	if (path == NULL)
+		goto bad;
+
+	do {
+		mib[miblen] = CTL_QUERY;
+		memset(&query, 0, sizeof(query));
+		query.sysctl_flags = SYSCTL_VERSION;
+
+		if (sysctl(mib, miblen+1, result, &res_size, &query,
+		    sizeof(query)) == -1) {
+			if (errno != ENOMEM)
+				goto bad1;
+			/* Grow up result */
+			newresult = (struct sysctlnode *)realloc(result, res_size);
+			if (newresult == NULL)
+				goto bad1;
+			result = newresult;
+			if (sysctl(mib, miblen+1, result, &res_size, &query,
+			    sizeof(query)) == -1)
+				goto bad1;
+		}
+		n = res_size / sizeof(struct sysctlnode);
+
+		while (*path == '/' || *path == '.')
+			path++;
+		node = strsep(&path, "./");
+
+		for (i = 0; i < n; i++)
+			if (!strcmp(result[i].sysctl_name, node)) {
+				mib[miblen] = result[i].sysctl_num;
+				miblen++;
+				break;
+			}
+	} while (path != NULL && miblen <= CTL_MAXNAME);
+
+	if (path != NULL)
+		goto bad1;
+	r = SYSCTL_TYPE(result[i].sysctl_flags);
+
+	free(path);
+	free(result);
+	if (sysctl(mib, miblen, oldp, oldlen, NULL, 0) == -1)
+		return (-1);
+	return r;
+
+bad1:
+	free(path);
+bad:
+	free(result);
+	return (-1);
+}
+
 static bool
 _rtld_load_by_name(const char *name, Obj_Entry *obj, Needed_Entry **needed, int mode)
 {
@@ -179,6 +250,7 @@
 	bool got = false;
 	union {
 		int i;
+		u_quad_t q;
 		char s[16];
 	} val;
 
@@ -187,22 +259,24 @@
 		if (strcmp(x->name, name) != 0)
 			continue;
 
-		i = sizeof(val);
-
-		if (sysctl(x->ctl, x->ctlmax, &val, &i, NULL, 0) == -1) {
-			xwarnx(_PATH_LD_HINTS ": unknown sysctl for %s", name);
+		j = sizeof(val);
+		if ((i = _rtld_sysctl(x->ctlname, &val, &j)) == -1) {
+			xwarnx(_PATH_LD_HINTS ": invalid/unknown sysctl for %s (%d)",
+			    name, errno);
 			break;
 		}
 
-		switch (x->ctltype[x->ctlmax - 1]) {
+		switch (i) {
+		case CTLTYPE_QUAD:
+			xsnprintf(val.s, sizeof(val.s), "%" PRIu64, val.q);
+			break;
 		case CTLTYPE_INT:
 			xsnprintf(val.s, sizeof(val.s), "%d", val.i);
 			break;
 		case CTLTYPE_STRING:
 			break;
 		default:
-			xwarnx("unsupported sysctl type %d",
-			    x->ctltype[x->ctlmax - 1]);
+			xwarnx("unsupported sysctl type %d", (int)i);
 			break;
 		}
 
Index: paths.c
===================================================================
RCS file: /cvsroot/src/libexec/ld.elf_so/paths.c,v
retrieving revision 1.30
diff -u -r1.30 paths.c
--- paths.c	16 Mar 2004 05:25:12 -0000	1.30
+++ paths.c	28 Jun 2004 22:27:34 -0000
@@ -39,7 +39,6 @@
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/param.h>
-#include <sys/sysctl.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/gmon.h>
@@ -61,7 +60,6 @@
 static const char *getstr(const char **, const char *, const char *);
 static const char *getcstr(const char **, const char *, const char *);
 static const char *getword(const char **, const char *, const char *);
-static int matchstr(const char *, const char *, const char *);
 
 static const char WS[] = " \t\n";
 
@@ -138,21 +136,6 @@
 	return (getstr(p, ep, delim));
 }
 
-/*
- * Match `bp' against NUL terminated string pointed by `p'.
- */
-static int
-matchstr(const char *p, const char *bp, const char *ep)
-{
-	int c;
-
-	while (bp < ep)
-		if ((c = *p++) == 0 || c != *bp++)
-			return (0);
-
-	return (*p == 0);
-}
-
 static Search_Path *
 _rtld_find_path(Search_Path *path, const char *pathstr, size_t pathlen)
 {
@@ -216,49 +199,6 @@
 	}
 }
 
-struct list {
-	const struct ctlname *ctl;
-	int numentries;
-};
-
-#ifdef CTL_MACHDEP_NAMES
-static const struct ctlname ctl_machdep[] = CTL_MACHDEP_NAMES;
-#endif
-static const struct ctlname ctl_toplvl[] = CTL_NAMES;
-
-const struct list toplevel[] = {
-	{ 0, 0 },
-	{ ctl_toplvl, CTL_MAXID },
-	{ 0, -1 },
-};
-
-const struct list secondlevel[] = {
-	{ 0, 0 },			/* CTL_UNSPEC */
-	{ 0, KERN_MAXID },		/* CTL_KERN */
-	{ 0, VM_MAXID },		/* CTL_VM */
-	{ 0, VFS_MAXID },		/* CTL_VFS */
-	{ 0, NET_MAXID },		/* CTL_NET */
-	{ 0, CTL_DEBUG_MAXID },		/* CTL_DEBUG */
-	{ 0, HW_MAXID },		/* CTL_HW */
-#ifdef CTL_MACHDEP_NAMES
-	{ ctl_machdep, CPU_MAXID },	/* CTL_MACHDEP */
-#else
-	{ 0, 0 },			/* CTL_MACHDEP */
-#endif
-	{ 0, USER_MAXID },		/* CTL_USER_NAMES */
-	{ 0, DDBCTL_MAXID },		/* CTL_DDB_NAMES */
-	{ 0, 2 },			/* dummy name */
-	{ 0, -1 },
-};
-
-const struct list *lists[] = {
-	toplevel,
-	secondlevel,
-	0
-};
-
-#define CTL_MACHDEP_SIZE (sizeof(ctl_machdep) / sizeof(ctl_machdep[0]))
-
 /*
  * Process library mappings of the form:
  *	<library_name>	<machdep_variable> <value,...:library_name,...> ... 
@@ -268,7 +208,7 @@
 {
 	Library_Xform *hwptr = NULL;
 	const char *ptr, *key, *ekey, *lib, *elib, *l;
-	int i, j, k;
+	int i, j;
 	
 	dbg((" processing mapping \"%.*s\"", (int)(ep - bp), bp));
 
@@ -290,37 +230,7 @@
 
 	dbg((" sysctl \"%.*s\"", (int)(bp - ptr), ptr));
 
-	for (i = 0; (l = getstr(&ptr, bp, ".")) != NULL; i++, ptr++) {
-
-		if (lists[i] == NULL || i >= RTLD_MAX_CTL) {
-			xwarnx("sysctl nesting too deep");
-			goto cleanup;
-		}
-
-		for (j = 1; lists[i][j].numentries != -1; j++) {
-
-			if (lists[i][j].ctl == NULL)
-				continue;
-
-			for (k = 1; k < lists[i][j].numentries; k++)
-				if (matchstr(lists[i][j].ctl[k].ctl_name, l,
-				    ptr))
-					break;
-
-			if (lists[i][j].numentries == -1) {
-				xwarnx("unknown sysctl variable name `%.*s'",
-				    (int)(ptr - l), l);
-				goto cleanup;
-			}
-
-			hwptr->ctl[hwptr->ctlmax] = k;
-			hwptr->ctltype[hwptr->ctlmax++] =
-			    lists[i][j].ctl[k].ctl_type;
-		}
-	}
-
-	for (i = 0; i < hwptr->ctlmax; i++)
-		dbg((" sysctl %d, %d", hwptr->ctl[i], hwptr->ctltype[i]));
+	hwptr->ctlname = exstrdup(ptr, bp);
 
 	for (i = 0; bp++, (ptr = getword(&bp, ep, WS)) != NULL;) {
 		dbg((" ptr = %.*s", (int)(bp - ptr), ptr));
Index: rtld.h
===================================================================
RCS file: /cvsroot/src/libexec/ld.elf_so/rtld.h,v
retrieving revision 1.70
diff -u -r1.70 rtld.h
--- rtld.h	12 Aug 2003 09:18:49 -0000	1.70
+++ rtld.h	28 Jun 2004 22:27:34 -0000
@@ -97,9 +97,7 @@
 typedef struct _rtld_library_xform_t {
 	struct _rtld_library_xform_t *next;
 	char *name;
-	int ctl[RTLD_MAX_CTL];
-	int ctltype[RTLD_MAX_CTL];
-	int ctlmax;
+	char *ctlname;
 	struct {
 		char *value;
 		char *library[RTLD_MAX_LIBRARY];

--Multipart=_Tue__29_Jun_2004_01_03_17_+0200_Wp3mnT6sjBqFtcE+--