Re: [PATCH] cleanup udevstart

[Kay Sievers <kay.sievers@vrfy.org>]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

On Tue, Mar 16, 2004 at 02:28:56AM +0100, Kay Sievers wrote:
> On Mon, Mar 15, 2004 at 10:22:02PM +0100, Olaf Hering wrote:
> >  On Tue, Mar 02, Greg KH wrote:
> > 
> > > # /usr/bin/time /sbin/udevstart 
> > > Command exited with non-zero status 22
> > > 0.19user 0.67system 0:01.04elapsed 83%CPU (0avgtext+0avgdata 0maxresident)k
> > > 0inputs+0outputs (2major+6065minor)pagefaults 0swaps
> > > 
> > > 1 second to populate a full /dev while the system is under a very heavy
> > > load is pretty good I think :)
> > 
> > 
> > Yeah, I was using the klibc version, that makes it slower due to the
> > clever fgets implementation.
> 
> Hey, better try to fix it :)
> You may try this one.

Here are all fgets() converted to mmap(), also the config file read.
I get for a very small(20 lines) rules file:

  time ./udevstart

  klibc mmap
  real    0m0.119s
  user    0m0.020s
  sys     0m0.040s

  glibc mmap
  real    0m0.232s
  user    0m0.064s
  sys     0m0.108s

  glibc fgets
  real    0m0.308s
  user    0m0.124s
  sys     0m0.123s

  klibc fgets
  real    0m0.341s
  user    0m0.086s
  sys     0m0.195s


The strace looks really short now :)

  [pid 28262] sigaction(SIGTERM, {0x80480c4, [], SA_RESTART}, NULL, 0x804e0ec) = 0
  [pid 28262] stat("/etc/udev/udev.rules", {st_mode=S_IFREG|0644, st_size=2485, ...}) = 0
  [pid 28262] open("/etc/udev/udev.rules", O_RDONLY) = 6
  [pid 28262] fstat(6, {st_mode=S_IFREG|0644, st_size=2485, ...}) = 0
  [pid 28262] mmap2(NULL, 2485, PROT_READ, MAP_SHARED, 6, 0) = 0x4001a000
  [pid 28262] munmap(0x4001a000, 2485)    = 0
  [pid 28262] stat("/etc/udev/udev.permissions", {st_mode=S_IFREG|0644, st_size=148, ...}) = 0



thanks,
Kay

[-- Attachment #2: 80-mmap.patch --]
[-- Type: text/plain, Size: 5942 bytes --]

===== klibc_fixups.c 1.7 vs edited =====
--- 1.7/klibc_fixups.c	Tue Mar  2 00:59:53 2004
+++ edited/klibc_fixups.c	Tue Mar 16 03:30:35 2004
@@ -28,6 +28,7 @@
 #include <fcntl.h>
 #include <sys/types.h>
 
+#include "udev.h"
 #include "klibc_fixups.h"
 #include "logging.h"
 
@@ -40,22 +41,35 @@
 static unsigned long get_id_by_name(const char *uname, const char *dbfile)
 {
 	unsigned long id = -1;
-	FILE *file;
-	char buf[255];
+	char line[255];
+	char *buf;
+	size_t bufsize;
+	size_t cur;
+	size_t count;
 	char *pos;
 	char *name;
 	char *idstr;
 	char *tail;
 
-	file = fopen(dbfile, "r");
-	if (file == NULL) {
-		dbg("unable to open file '%s'", dbfile);
+	if (file_map(dbfile, &buf, &bufsize) == 0) {
+		dbg("reading '%s' as db file", dbfile);
+	} else {
+		dbg("can't open '%s' as db file", dbfile);
 		return -1;
 	}
 
+	/* loop through the whole file */
+
+	cur = 0;
 	while (1) {
-		pos = fgets(buf, sizeof(buf), file);
-		if (pos == NULL)
+		count = buf_get_line(buf, bufsize, cur);
+
+		strncpy(line, buf + cur, count);
+		line[count] = '\0';
+		pos = line;
+
+		cur += count+1;
+		if (cur > bufsize)
 			break;
 
 		/* get name */
@@ -82,7 +96,7 @@
 		}
 	}
 
-	fclose(file);
+	file_unmap(buf, bufsize);
 	return id;
 }
 
===== namedev_parse.c 1.32 vs edited =====
--- 1.32/namedev_parse.c	Fri Mar 12 17:43:30 2004
+++ edited/namedev_parse.c	Tue Mar 16 03:31:03 2004
@@ -30,7 +30,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
-#include <fcntl.h>
 #include <ctype.h>
 #include <unistd.h>
 #include <sys/stat.h>
@@ -153,27 +152,36 @@
 	char *temp2;
 	char *temp3;
 	char *attr;
-	FILE *fd;
+	char *buf;
+	size_t bufsize;
+	size_t cur;
+	size_t count;
 	int program_given = 0;
 	int retval = 0;
 	struct config_device dev;
 
-	fd = fopen(filename, "r");
-	if (fd != NULL) {
+	if (file_map(filename, &buf, &bufsize) == 0) {
 		dbg("reading '%s' as rules file", filename);
 	} else {
-		dbg("can't open '%s' as a rules file", filename);
-		return -ENODEV;
+		dbg("can't open '%s' as rules file", filename);
+		return -1;
 	}
 
 	/* loop through the whole file */
+	cur = 0;
 	lineno = 0;
 	while (1) {
-		/* get a line */
-		temp = fgets(line, sizeof(line), fd);
-		if (temp == NULL)
-			goto exit;
+		count = buf_get_line(buf, bufsize, cur);
+
+		strncpy(line, buf + cur, count);
+		line[count] = '\0';
+		temp = line;
 		lineno++;
+
+		cur += count+1;
+		if (cur > bufsize)
+			break;
+
 		dbg_parse("read '%s'", temp);
 
 		/* eat the whitespace */
@@ -311,8 +319,8 @@
 			    filename, lineno, temp - line);
 		}
 	}
-exit:
-	fclose(fd);
+
+	file_unmap(buf, bufsize);
 	return retval;
 }
 
@@ -321,22 +329,31 @@
 	char line[255];
 	char *temp;
 	char *temp2;
-	FILE *fd;
+	char *buf;
+	size_t bufsize;
+	size_t cur;
+	size_t count;
 	int retval = 0;
 	struct perm_device dev;
 
-	fd = fopen(filename, "r");
-	if (fd != NULL) {
+	if (file_map(filename, &buf, &bufsize) == 0) {
 		dbg("reading '%s' as permissions file", filename);
 	} else {
 		dbg("can't open '%s' as permissions file", filename);
-		return -ENODEV;
+		return -1;
 	}
 
 	/* loop through the whole file */
+	cur = 0;
 	while (1) {
-		temp = fgets(line, sizeof(line), fd);
-		if (temp == NULL)
+		count = buf_get_line(buf, bufsize, cur);
+
+		strncpy(line, buf + cur, count);
+		line[count] = '\0';
+		temp = line;
+
+		cur += count+1;
+		if (cur > bufsize)
 			break;
 
 		dbg_parse("read '%s'", temp);
@@ -394,7 +411,7 @@
 	}
 
 exit:
-	fclose(fd);
+	file_unmap(buf, bufsize);
 	return retval;
 }
 
===== udev.h 1.51 vs edited =====
--- 1.51/udev.h	Thu Mar  4 22:40:39 2004
+++ edited/udev.h	Tue Mar 16 03:30:36 2004
@@ -27,7 +27,10 @@
 #include <string.h>
 #include <sysfs/libsysfs.h>
 #include <stddef.h>
+#include <fcntl.h>
 #include <sys/param.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
 
 #define COMMENT_CHARACTER		'#'
 
@@ -140,6 +143,43 @@
 		subsystem[SUBSYSTEM_SIZE-1] = '\0';
 
 	return subsystem;
+}
+
+static inline int file_map(const char *filename, char **buf, size_t *bufsize)
+{
+	struct stat stats;
+	int fd;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		return -1;
+	}
+
+	if (fstat(fd, &stats) < 0) {
+		return -1;
+	}
+
+	*buf = mmap(NULL, stats.st_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (*buf == MAP_FAILED) {
+		return -1;
+	}
+	*bufsize = stats.st_size;
+
+	return 0;
+}
+
+static inline void file_unmap(char *buf, size_t bufsize)
+{
+	munmap(buf, bufsize);
+}
+
+static inline size_t buf_get_line(char *buf, size_t buflen, size_t cur)
+{
+	size_t count = 0;
+
+	for (count = cur; count < buflen && buf[count] != '\n'; count++);
+
+	return count - cur;
 }
 
 extern int udev_add_device(char *path, char *subsystem, int fake);
===== udev_config.c 1.14 vs edited =====
--- 1.14/udev_config.c	Thu Mar  4 22:40:39 2004
+++ edited/udev_config.c	Tue Mar 16 03:30:37 2004
@@ -131,12 +131,14 @@
 	char *temp;
 	char *variable;
 	char *value;
-	FILE *fd;
-	int lineno = 0;
+	char *buf;
+	size_t bufsize;
+	size_t cur;
+	size_t count;
+	int lineno;
 	int retval = 0;
-	
-	fd = fopen(udev_config_filename, "r");
-	if (fd != NULL) {
+
+	if (file_map(udev_config_filename, &buf, &bufsize) == 0) {
 		dbg("reading '%s' as config file", udev_config_filename);
 	} else {
 		dbg("can't open '%s' as config file", udev_config_filename);
@@ -144,13 +146,20 @@
 	}
 
 	/* loop through the whole file */
+	lineno = 0;
+	cur = 0;
 	while (1) {
-		/* get a line */
-		temp = fgets(line, sizeof(line), fd);
-		if (temp == NULL)
-			goto exit;
+		count = buf_get_line(buf, bufsize, cur);
+
+		strncpy(line, buf + cur, count);
+		line[count] = '\0';
+		temp = line;
 		lineno++;
 
+		cur += count+1;
+		if (cur > bufsize)
+			break;
+
 		dbg_parse("read '%s'", temp);
 
 		/* eat the whitespace at the beginning of the line */
@@ -182,8 +191,8 @@
 	}
 	dbg_parse("%s:%d:%Zd: error parsing '%s'", udev_config_filename,
 		  lineno, temp - line, temp);
-exit:
-	fclose(fd);
+
+	file_unmap(buf, bufsize);
 	return retval;
 }