PATCH udev-106 /proc/stat buffer to small

PATCH udev-106 /proc/stat buffer to small

[George Beshers]

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


I am assuming that this is the correct place to post udev patches for
consideration, feel free to inform me otherwise :-).

George


[-- Attachment #2: udev.patch --]
[-- Type: text/x-patch, Size: 2018 bytes --]

This came up on a system with 1024cores (well 768cpus and 256
hyper-threads) where the buffer size for reading /proc/stat was
not large enough causing udev to fail during the boot process.
The read_proc_stat(void) eliminates this problem.

routine makes sure that the 

--- udev-105/udevd.c	2007-02-02 19:24:48.000000000 -0500
+++ udev-105.new/udevd.c	2007-02-19 13:47:54.000000000 -0500
@@ -357,25 +357,58 @@
 	return memsize / 1024;
 }
 
-static int cpu_count(void)
+/*
+ * Use the same buffer for cpu_count() and running_processes()
+ */
+
+static char* stat_buf = NULL;
+static int   stat_buf_size = 4096;
+
+static int read_proc_stat(void)
 {
 	int f;
-	char buf[32768];
 	int len;
-	const char *pos;
-	int count = 0;
 
-	f = open("/proc/stat", O_RDONLY);
-	if (f == -1)
-		return -1;
+	if (stat_buf == NULL) {
+		stat_buf = malloc(stat_buf_size);
+		if (stat_buf == NULL)
+			return -1;
+	}
 
-	len = read(f, buf, sizeof(buf)-1);
+	do {
+		f = open("/proc/stat", O_RDONLY);
+		if (f == -1)
+			return -1;
+
+		len = read(f, stat_buf, stat_buf_size-1);
+		if (len < stat_buf_size-1)
+			break;
+
+		stat_buf_size *= 2;
+		stat_buf = realloc(stat_buf, stat_buf_size);
+		if (stat_buf == NULL)
+			return -1;
+	} while (1);
 	close(f);
+
 	if (len <= 0)
 		return -1;
-	buf[len] = '\0';
+	stat_buf[len] = '\0';
+	return len;
+}
+
 
-	pos = strstr(buf, "cpu");
+static int cpu_count(void)
+{
+	int len;
+	const char *pos;
+	int count = 0;
+
+	len = read_proc_stat();
+	if (len <= 0)
+		return -1;
+
+	pos = strstr(stat_buf, "cpu");
 	if (pos == NULL)
 		return -1;
 
@@ -392,23 +425,15 @@
 
 static int running_processes(void)
 {
-	int f;
-	char buf[32768];
 	int len;
 	int running;
 	const char *pos;
 
-	f = open("/proc/stat", O_RDONLY);
-	if (f == -1)
-		return -1;
-
-	len = read(f, buf, sizeof(buf)-1);
-	close(f);
+	len = read_proc_stat();
 	if (len <= 0)
 		return -1;
-	buf[len] = '\0';
 
-	pos = strstr(buf, "procs_running ");
+	pos = strstr(stat_buf, "procs_running ");
 	if (pos == NULL)
 		return -1;
 

[-- Attachment #3: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 226 bytes --]

_______________________________________________
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

Re: PATCH udev-106 /proc/stat buffer to small

[George Beshers]

sigh...

Never answer the phone in the middle of preparing a post :-(
s/to/too/ is subject

Delete:
> routine makes sure that the 
>
>   
The actual patch should be good :-).

George


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
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

Re: PATCH udev-106 /proc/stat buffer to small

[Kay Sievers]

On 3/5/07, George Beshers <gbeshers@sgi.com> wrote:
>
> I am assuming that this is the correct place to post udev patches for
> consideration, feel free to inform me otherwise :-).

> This came up on a system with 1024cores (well 768cpus and 256
> hyper-threads) where the buffer size for reading /proc/stat was
> not large enough causing udev to fail during the boot process.

"fail"? What fails? How? It should still find hundreds of your CPU's, right?

> The read_proc_stat(void) eliminates this problem.

Just curious, what size is the content of /proc/stat on this box?

Thanks,
Kay

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
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

Re: PATCH udev-106 /proc/stat buffer to small

[George Beshers]

First, I will post an improved patch as I completely forgot udev
was threaded (duh..).

Kay Sievers wrote:
> On 3/5/07, George Beshers <gbeshers@sgi.com> wrote:
>>
>> I am assuming that this is the correct place to post udev patches for
>> consideration, feel free to inform me otherwise :-).
>
>> This came up on a system with 1024cores (well 768cpus and 256
>> hyper-threads) where the buffer size for reading /proc/stat was
>> not large enough causing udev to fail during the boot process.
>
> "fail"? What fails? How? It should still find hundreds of your CPU's, 
> right?
Yes and no as it turns out because udev also looks at procs_running 
which is at
the end of the list.  If the buffer isn't big enough it returns an 
error, does a lame
boot, and puts FAILED rather than OK in the syslog.
>
>> The read_proc_stat(void) eliminates this problem.
>
> Just curious, what size is the content of /proc/stat on this box?
Aprox 48000 is my memory -- well above the 32768 that the buffer size was.
Really want this to scale to 4k or so without doing anything terrible to the
1p 128meg systems that still exist.
>
> Thanks,
> Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
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

Re: PATCH udev-106 /proc/stat buffer to small

[Kay Sievers]

On Tue, 2007-03-06 at 10:15 -0500, George Beshers wrote:
> First, I will post an improved patch as I completely forgot udev
> was threaded (duh..).

Threaded?

> Kay Sievers wrote:
> > On 3/5/07, George Beshers <gbeshers@sgi.com> wrote:
> >>
> >> I am assuming that this is the correct place to post udev patches for
> >> consideration, feel free to inform me otherwise :-).
> >
> >> This came up on a system with 1024cores (well 768cpus and 256
> >> hyper-threads) where the buffer size for reading /proc/stat was
> >> not large enough causing udev to fail during the boot process.
> >
> > "fail"? What fails? How? It should still find hundreds of your CPU's, 
> > right?
> Yes and no as it turns out because udev also looks at procs_running 
> which is at
> the end of the list.  If the buffer isn't big enough it returns an 
> error, does a lame
> boot, and puts FAILED rather than OK in the syslog.
> >
> >> The read_proc_stat(void) eliminates this problem.
> >
> > Just curious, what size is the content of /proc/stat on this box?
> Aprox 48000 is my memory -- well above the 32768 that the buffer size was.
> Really want this to scale to 4k or so without doing anything terrible to the
> 1p 128meg systems that still exist.

Well, I rather switch to fgets(), that should leave all that buffered IO
crap to glibc. As we are reading a seq proc-file here, it doesn't really make
sense to put the whole file into a local buffer before reading it, right?

Thanks,
Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
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