* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
@ 2004-03-02 22:16 ` Greg KH
2004-03-02 23:09 ` Olaf Hering
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2004-03-02 22:16 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 02, 2004 at 10:55:36PM +0100, Kay Sievers wrote:
> I just wanted to terminate the snprintf() strings, cause I can see a
> overflow with closed eyes after all the audit :)
>
> But then I changed a bit more to bring it in line with the style of the
> other files. I replaced the exec_udev() function with the one from
> udevd, cause we don't need to read the stdout from udev.
>
> Please have a look if it still works for you too and not
> only for usernames with 3 characters :)
Ah, much nicer, thanks a lot for doing this. It works in my testing,
and I've applied it.
greg k-h
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
2004-03-02 22:16 ` Greg KH
@ 2004-03-02 23:09 ` Olaf Hering
2004-03-02 23:20 ` Greg KH
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Olaf Hering @ 2004-03-02 23:09 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 02, Kay Sievers wrote:
> +#define SYSBLOCK "/sys/block"
> +#define SYSCLASS "/sys/class"
> +#define UDEV_BIN "/sbin/udev"
> pid = fork();
> - retval = execl("/sbin/udev", "/sbin/udev", path, NULL);
I have not looked closer at it, wouldnt it be better to feed the udevdb
directly, instead of spawning countless udev processes? Some sort of
udev binary which takes optional directories as agument to limit it to
these sysfs dirs.
--
USB is for mice, FireWire is for men!
sUse lINUX ag, n√úRNBERG
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&opÃk
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
2004-03-02 22:16 ` Greg KH
2004-03-02 23:09 ` Olaf Hering
@ 2004-03-02 23:20 ` Greg KH
2004-03-02 23:23 ` Olaf Hering
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2004-03-02 23:20 UTC (permalink / raw)
To: linux-hotplug
On Wed, Mar 03, 2004 at 12:09:51AM +0100, Olaf Hering wrote:
> On Tue, Mar 02, Kay Sievers wrote:
>
> > +#define SYSBLOCK "/sys/block"
> > +#define SYSCLASS "/sys/class"
> > +#define UDEV_BIN "/sbin/udev"
>
> > pid = fork();
>
> > - retval = execl("/sbin/udev", "/sbin/udev", path, NULL);
>
> I have not looked closer at it, wouldnt it be better to feed the udevdb
> directly, instead of spawning countless udev processes? Some sort of
> udev binary which takes optional directories as agument to limit it to
> these sysfs dirs.
Nah, it's simpler this way. We spawn and then wait, so there isn't a
zillion udev processes all running at once :)
thanks,
greg k-h
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (2 preceding siblings ...)
2004-03-02 23:20 ` Greg KH
@ 2004-03-02 23:23 ` Olaf Hering
2004-03-02 23:32 ` Greg KH
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Olaf Hering @ 2004-03-02 23:23 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 02, Greg KH wrote:
> On Wed, Mar 03, 2004 at 12:09:51AM +0100, Olaf Hering wrote:
> > On Tue, Mar 02, Kay Sievers wrote:
> >
> > > +#define SYSBLOCK "/sys/block"
> > > +#define SYSCLASS "/sys/class"
> > > +#define UDEV_BIN "/sbin/udev"
> >
> > > pid = fork();
> >
> > > - retval = execl("/sbin/udev", "/sbin/udev", path, NULL);
> >
> > I have not looked closer at it, wouldnt it be better to feed the udevdb
> > directly, instead of spawning countless udev processes? Some sort of
> > udev binary which takes optional directories as agument to limit it to
> > these sysfs dirs.
>
> Nah, it's simpler this way. We spawn and then wait, so there isn't a
> zillion udev processes all running at once :)
thats the point, wait for what?
I hope the whole point is to create all nodes in a jiffy, in the long run?
--
USB is for mice, FireWire is for men!
sUse lINUX ag, n√úRNBERG
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&opÃk
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (3 preceding siblings ...)
2004-03-02 23:23 ` Olaf Hering
@ 2004-03-02 23:32 ` Greg KH
2004-03-15 21:22 ` Olaf Hering
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2004-03-02 23:32 UTC (permalink / raw)
To: linux-hotplug
On Wed, Mar 03, 2004 at 12:23:56AM +0100, Olaf Hering wrote:
> On Tue, Mar 02, Greg KH wrote:
>
> > On Wed, Mar 03, 2004 at 12:09:51AM +0100, Olaf Hering wrote:
> > > On Tue, Mar 02, Kay Sievers wrote:
> > >
> > > > +#define SYSBLOCK "/sys/block"
> > > > +#define SYSCLASS "/sys/class"
> > > > +#define UDEV_BIN "/sbin/udev"
> > >
> > > > pid = fork();
> > >
> > > > - retval = execl("/sbin/udev", "/sbin/udev", path, NULL);
> > >
> > > I have not looked closer at it, wouldnt it be better to feed the udevdb
> > > directly, instead of spawning countless udev processes? Some sort of
> > > udev binary which takes optional directories as agument to limit it to
> > > these sysfs dirs.
> >
> > Nah, it's simpler this way. We spawn and then wait, so there isn't a
> > zillion udev processes all running at once :)
>
> thats the point, wait for what?
> I hope the whole point is to create all nodes in a jiffy, in the long run?
At startup we have to wait as usually the next thing the initscripts do
is use those device nodes. So we have to wait until they are all
created.
And it's fast today. Have you timed udevstart (or even the start_udev
bash script)? Here's my box as I do a kernel build in the background at
the same time:
# /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 :)
thanks,
greg k-h
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (4 preceding siblings ...)
2004-03-02 23:32 ` Greg KH
@ 2004-03-15 21:22 ` Olaf Hering
2004-03-16 1:28 ` Kay Sievers
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Olaf Hering @ 2004-03-15 21:22 UTC (permalink / raw)
To: linux-hotplug
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.
--
USB is for mice, FireWire is for men!
sUse lINUX ag, n√úRNBERG
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&opÃk
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (5 preceding siblings ...)
2004-03-15 21:22 ` Olaf Hering
@ 2004-03-16 1:28 ` Kay Sievers
2004-03-16 2:49 ` Kay Sievers
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kay Sievers @ 2004-03-16 1:28 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 604 bytes --]
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.
thanks,
Kay
[-- Attachment #2: 80-mmap.patch --]
[-- Type: text/plain, Size: 2189 bytes --]
===== 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 02:16:35 2004
@@ -36,6 +36,7 @@
#include <sys/stat.h>
#include <dirent.h>
#include <errno.h>
+#include <sys/mman.h>
#include "udev.h"
#include "logging.h"
@@ -145,6 +146,47 @@
return NULL;
}
+static int file_map(const char *filename, char **buf, size_t *bufsize)
+{
+ struct stat stats;
+ int fd;
+
+ fd = open(filename, O_RDONLY);
+ if (fd < 0) {
+ dbg("can't open '%s'", filename);
+ return -1;
+ }
+
+ if (fstat(fd, &stats) < 0) {
+ dbg("error stating '%s'", filename);
+ return -1;
+ }
+
+ *buf = mmap(NULL, stats.st_size, PROT_READ, MAP_SHARED, fd, 0);
+ if (*buf == MAP_FAILED) {
+ dbg("error mapping file '%s'", filename);
+ return -1;
+ }
+ *bufsize = stats.st_size;
+
+ return 0;
+}
+
+static void file_unmap(char *buf, size_t bufsize)
+{
+ munmap(buf, bufsize);
+}
+
+
+static 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;
+}
+
static int namedev_parse_rules(char *filename)
{
char line[255];
@@ -153,27 +195,38 @@
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("bufsize %i", bufsize);
+
/* 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 */
@@ -312,7 +365,7 @@
}
}
exit:
- fclose(fd);
+ file_unmap(buf, bufsize);
return retval;
}
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (6 preceding siblings ...)
2004-03-16 1:28 ` Kay Sievers
@ 2004-03-16 2:49 ` Kay Sievers
2004-03-16 17:01 ` Patrick Mansfield
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kay Sievers @ 2004-03-16 2:49 UTC (permalink / raw)
To: linux-hotplug
[-- 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;
}
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (7 preceding siblings ...)
2004-03-16 2:49 ` Kay Sievers
@ 2004-03-16 17:01 ` Patrick Mansfield
2004-03-16 21:57 ` Kay Sievers
2004-03-16 22:24 ` Patrick Mansfield
10 siblings, 0 replies; 12+ messages in thread
From: Patrick Mansfield @ 2004-03-16 17:01 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 16, 2004 at 03:49:51AM +0100, Kay Sievers wrote:
>
> Here are all fgets() converted to mmap(), also the config file read.
> I get for a very small(20 lines) rules file:
>
> time ./udevstart
My timings using lots of scsi_debug LUN's and 1000 line udev.rules got a
about twice the speedup for klibc compared to your timings (almost 6 times
faster). None of my udev.rules match, so all 1000 rules are compared even
for 512 LUNs.
The inlines of longer functions gains nothing (and probably the other
inlines in udev.h that are more than 2 lines long and use more than once).
Not inlining file_map and buf_get_line gives almost no difference in
times, but shrinks executables a bit.
insmod was really this:
modprobe scsi_debug num_tgts2 max_luns\x16 delay=0
Total elapsed times as follows:
insmod rmmod
klibc current 4:11.42 4:06.27
klibc patch 0:38.48 0:30.89
klibc noinline 0:38.33 0:30.89
glibc current 0:40.84 0:36.20
glibc with patch 0:40.72 0:32.41
And:
# size */udev */udevtest */udevinfo
text data bss dec hex filename
64888 264 22512 87664 15670 noinline-udev/udev
65032 264 22512 87808 15700 udev-bk/udev
64044 260 22480 86784 15300 noinline-udev/udevtest
64188 260 22480 86928 15390 udev-bk/udevtest
68900 236 22524 91660 1660c noinline-udev/udevinfo
53092 140 22044 75276 1260c udev-bk/udevinfo
-- Patrick Mansfield
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (8 preceding siblings ...)
2004-03-16 17:01 ` Patrick Mansfield
@ 2004-03-16 21:57 ` Kay Sievers
2004-03-16 22:24 ` Patrick Mansfield
10 siblings, 0 replies; 12+ messages in thread
From: Kay Sievers @ 2004-03-16 21:57 UTC (permalink / raw)
To: linux-hotplug
On Tue, 2004-03-16 at 18:01, Patrick Mansfield wrote:
> On Tue, Mar 16, 2004 at 03:49:51AM +0100, Kay Sievers wrote:
> >
> > Here are all fgets() converted to mmap(), also the config file read.
> > I get for a very small(20 lines) rules file:
> >
> > time ./udevstart
>
> My timings using lots of scsi_debug LUN's and 1000 line udev.rules got a
> about twice the speedup for klibc compared to your timings (almost 6 times
> faster). None of my udev.rules match, so all 1000 rules are compared even
> for 512 LUNs.
Hey, looks like a pretty nice speedup.
> The inlines of longer functions gains nothing (and probably the other
> inlines in udev.h that are more than 2 lines long and use more than once).
> Not inlining file_map and buf_get_line gives almost no difference in
> times, but shrinks executables a bit.
Yes, I know. It was just a quick hack. I tried to avoid putting "normal"
functions into a header file. If we want this to go in, we may create a
udev_lib.[hc] or something and put in all the generic stuff currently in
udev.h.
> insmod was really this:
>
> modprobe scsi_debug num_tgts2 max_luns\x16 delay=0
>
> Total elapsed times as follows:
>
> insmod rmmod
> klibc current 4:11.42 4:06.27
> klibc patch 0:38.48 0:30.89
> klibc noinline 0:38.33 0:30.89
> glibc current 0:40.84 0:36.20
> glibc with patch 0:40.72 0:32.41
Much thanks for your measurements. It's always better to have real
numbers :)
It seeems that we need to fix it. The mmap() difference for glibc is
pretty small, so it should also be possible to fix the fgets() in klibc.
Any suggestion to go with mmap() or fix klibc instead?
thanks again,
Kay
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cleanup udevstart
2004-03-02 21:55 [PATCH] cleanup udevstart Kay Sievers
` (9 preceding siblings ...)
2004-03-16 21:57 ` Kay Sievers
@ 2004-03-16 22:24 ` Patrick Mansfield
10 siblings, 0 replies; 12+ messages in thread
From: Patrick Mansfield @ 2004-03-16 22:24 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 16, 2004 at 10:57:55PM +0100, Kay Sievers wrote:
> On Tue, 2004-03-16 at 18:01, Patrick Mansfield wrote:
> > On Tue, Mar 16, 2004 at 03:49:51AM +0100, Kay Sievers wrote:
>
> Yes, I know. It was just a quick hack. I tried to avoid putting "normal"
> functions into a header file. If we want this to go in, we may create a
> udev_lib.[hc] or something and put in all the generic stuff currently in
> udev.h.
udev_lib sounds good, I created a udev-other.c.
> Much thanks for your measurements. It's always better to have real
> numbers :)
Well they are still not real disks. I assume we have even less of an issue
using real disks, as IO times increase and the hotplug and udev overhead
becomes a smaller percentage.
> It seeems that we need to fix it. The mmap() difference for glibc is
> pretty small, so it should also be possible to fix the fgets() in klibc.
> Any suggestion to go with mmap() or fix klibc instead?
IMO klibc should buffer fgets() no matter what udev uses.
So fix klibc and use its fgets, and if that can't happen soon use your
mmap patch.
-- Patrick Mansfield
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 12+ messages in thread