mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: "Ulrich Drepper" <drepper@gmail.com>
Cc: Arjan van de Ven <arjan@infradead.org>,
	"Randy.Dunlap" <rdunlap@xenotime.net>,
	akpm@osdl.org, linux-kernel@vger.kernel.org,
	libc-alpha@sourceware.org
Subject: [PATCH] Use uname not sysctl to get the kernel revision
Date: Wed, 12 Jul 2006 10:25:00 -0600	[thread overview]
Message-ID: <m1irm2bxk3.fsf_-_@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <1152601640.3128.7.camel@laptopd505.fenrus.org> (Arjan van de Ven's message of "Tue, 11 Jul 2006 09:07:20 +0200")


Currently it is felt but at least a subset of the kernel maintainers
that the binary sysctl interface is not maintainable, and the /proc
/sys interface should be used instead.  In investigating this it turns
out that the pthread code in glibc for detecting a SMP kernel appears
to be the primary user.

The information that we are asking for is available from the uname
system call so I don't understand why the code is using sysctl.

To understand the cost of the various approaches I put together
a little test program.  Using time for timing and running 100000
repetitions of the various system calls I get about
sysctl: 0.3s to 0.2s
uname:  0.1s to 0.07s
proc:   7.5 to  4.1s

proc is significantly slower which puzzles me.
But uname is noticeably faster than sysctl and uname is more portable
across linux flavors.  So updating the glibc pthread code to use
uname looks like the right way to implement is_smp_system. 

I do think detecting a SMP kernel to enable busy waiting on contended
mutexes is a very peculiar thing to be doing.  

My test performance test program:
> #include <string.h>
> #include <stdio.h>
> #include <sys/utsname.h>
> #include <errno.h>
> #include <stdarg.h>
> #include <stdlib.h>
> #include <sys/sysctl.h>
> #include <fcntl.h>
> #include <unistd.h>
> 
> static void uname_test(void)
> {
> 	struct utsname uts;
> 	uname(&uts);
> }
> 
> static void proc_test(void)
> {
> 	int fd;
> 	char buf[512];
> 	fd = open("/proc/sys/kernel/version", O_RDONLY);
> 	read(fd, buf, sizeof(buf));
> 	close(fd);
> }
> 
> static void sysctl_test(void)
> {
> 	static int sysctl_args[] = { CTL_KERN, KERN_VERSION };
> 	char buf[512];
> 	size_t reslen = sizeof(buf);
> 
> 	sysctl(sysctl_args, sizeof(sysctl_args)/sizeof(sysctl_args[0]),
> 		buf, &reslen, NULL, 0);
> }
> 
> int main(int argc, char *argv[])
> {
> 	void (*test)(void) = NULL;
> 	int reps = -1;
> 	int i;
> 
> 	for (i = 1; i < argc; i++) {
> 		if (strcmp(argv[i], "--sysctl") == 0)
> 			test = sysctl_test;
> 		else if (strcmp(argv[i], "--uname") == 0)
> 			test = uname_test;
> 		else if (strcmp(argv[i], "--proc") == 0)
> 			test = proc_test;
> 		else 
> 			reps = atol(argv[i]);
> 	}
> 	if ((reps == -1) || (test == NULL)) {
> 		fprintf(stderr, "usage: [--sysctl | --uname | --proc] <reps>\n");
> 		return 1;
> 	}
> 
> 	for (i = 0; i < reps; i++) {
> 		test();
> 	}
> 	return 0;
> }


My patch to use uname instead of proc or sysctl to get the 

--- glibc-2.4/nptl/sysdeps/unix/sysv/linux/smp.h-sysctl	2006-07-12 08:48:44.000000000 -0600
+++ glibc-2.4/nptl/sysdeps/unix/sysv/linux/smp.h	2006-07-12 09:57:07.000000000 -0600
@@ -17,11 +17,8 @@
    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    Boston, MA 02111-1307, USA.  */
 
-#include <errno.h>
-#include <fcntl.h>
 #include <string.h>
-#include <sys/sysctl.h>
-#include <not-cancel.h>
+#include <sys/utsname>
 
 /* Test whether the machine has more than one processor.  This is not the
    best test but good enough.  More complicated tests would require `malloc'
@@ -29,24 +26,8 @@
 static inline int
 is_smp_system (void)
 {
-  static const int sysctl_args[] = { CTL_KERN, KERN_VERSION };
-  char buf[512];
-  size_t reslen = sizeof (buf);
-
-  /* Try reading the number using `sysctl' first.  */
-  if (__sysctl ((int *) sysctl_args,
-		sizeof (sysctl_args) / sizeof (sysctl_args[0]),
-		buf, &reslen, NULL, 0) < 0)
-    {
-      /* This was not successful.  Now try reading the /proc filesystem.  */
-      int fd = open_not_cancel_2 ("/proc/sys/kernel/version", O_RDONLY);
-      if (__builtin_expect (fd, 0) == -1
-	  || (reslen = read_not_cancel (fd, buf, sizeof (buf))) <= 0)
-	/* This also didn't work.  We give up and say it's a UP machine.  */
-	buf[0] = '\0';
-
-      close_not_cancel_no_status (fd);
-    }
-
-  return strstr (buf, "SMP") != NULL;
+  struct utsname uts;
+  if (uname(&uts) < 0)
+	  uts.version[0] = '\0';
+  return strstr (uts.version, "SMP") != NULL;
 }

  reply	other threads:[~2006-07-12 16:26 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-10 22:39 [PATCH] sysctl: Document that sys_sysctl will be removed Eric W. Biederman
2006-07-10 22:50 ` Randy.Dunlap
2006-07-11  4:10   ` Eric W. Biederman
2006-07-11  7:07     ` Arjan van de Ven
2006-07-12 16:25       ` Eric W. Biederman [this message]
2006-07-12 16:50         ` [PATCH] Use uname not sysctl to get the kernel revision Ulrich Drepper
2006-07-12 17:42           ` Eric W. Biederman
2006-07-12 23:24             ` Theodore Tso
2006-07-12 23:31               ` Andi Kleen
2006-07-13  0:12                 ` Theodore Tso
2006-07-13  2:33                   ` Eric W. Biederman
2006-07-13 12:15                   ` Andi Kleen
2006-07-12 23:44               ` Steve Munroe
2006-07-14 18:49                 ` Benjamin Herrenschmidt
2006-07-14 19:09                   ` Andi Kleen
2006-07-13  0:19               ` Eric W. Biederman
2006-07-12 18:44           ` Roland McGrath
2006-07-12 19:33             ` Ulrich Drepper
2006-07-12 19:53               ` Jakub Jelinek
2006-07-12 20:09                 ` H. Peter Anvin
2006-07-12 21:23                   ` Eric W. Biederman
2006-07-12 21:29                     ` Arjan van de Ven
2006-07-12 21:56                       ` Eric W. Biederman
2006-07-12 22:01                         ` Arjan van de Ven
2006-07-12 22:02                           ` H. Peter Anvin
2006-07-12 22:26                             ` Eric W. Biederman
2006-07-12 22:31                               ` H. Peter Anvin
2006-07-12 23:07                               ` Alan Cox
2006-07-12 23:19                                 ` H. Peter Anvin
2006-07-13 11:15                                   ` Alan Cox
2006-07-14 18:45                                 ` Benjamin Herrenschmidt
2006-07-14 19:11                                   ` H. Peter Anvin
2006-07-12 21:29                     ` H. Peter Anvin
2006-07-12 21:33                     ` Michael Tokarev
2006-07-13  5:17                     ` Ulrich Drepper
2006-07-13  6:27                       ` Ian Wienand
2006-07-13 14:39                       ` Eric W. Biederman
2006-07-13 15:05                         ` Arjan van de Ven
2006-07-13  5:00 Albert Cahalan
2006-07-13  5:42 ` H. Peter Anvin
2006-07-13  6:09   ` Albert Cahalan
2006-07-13  6:13     ` Albert Cahalan
2006-07-13  6:38 ` Eric W. Biederman
2006-07-13 16:15   ` Albert Cahalan
2006-07-13 16:53     ` Eric W. Biederman
2006-07-13 17:06       ` Albert Cahalan
2006-07-13 15:20 ` Eric W. Biederman

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=m1irm2bxk3.fsf_-_@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@osdl.org \
    --cc=arjan@infradead.org \
    --cc=drepper@gmail.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@xenotime.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome