Re: udevd on very large systems: again

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

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

On Thu, 2007-05-03 at 01:42 +0200, Kay Sievers wrote:
> On Wed, 2007-05-02 at 17:26 -0400, George Beshers wrote:
> > Kay Sievers wrote:
> > > Looks fine so far, but I still don't see how reading a seq-proc-file
> > > could miss entries. Are you expecting this to happen? Did you ever see
> > > this for any of these files?
> 
> > The answer is slightly complex.  No I never saw it happen, but I am
> > concerned about it.  To illustrate that, consider the following test
> > program and the output log attached.  Basically, the /proc/stat file
> > does seem to be changing dynamically and an open file does not
> > present a stable snapshot.
> 
> Looking at the seq_file code, calling lseek() causes the buffer to be
> repopulated:
>   seq_lseek() -> traverse() -> m->op->show()
> while show() is the method of the "stat" file that fills the buffer.
> 
> A simple read() should not do that, and just copy the content (up to
> 128kB) of the kernel seq_file file buffer to the user-process:
>   seq_read() -> copy_to_user(buf, m->buf + m->from, n)
> 
> >  So the danger is that the procs_running
> > might get split at a 4k boundary and the split changes by the time
> > the next read occurs so procs_running is corrupted.
> 
> I'm pretty sure, that this doesn't happen, if you don't seek(). Here is
> a test, that opens /proc/stat, reads a single byte (which fills the
> buffer), forks a ton of processes and reads the rest of the buffer. You
> see that the "processes" value did not increase:
>   $ ./seq 
>   processes 57425
>   processes 57425
>   processes 58203

The patch should be as simple as this now, I hope. :)

Thanks,
Kay

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

diff --git a/udevd.c b/udevd.c
index 645b068..4086b96 100644
--- a/udevd.c
+++ b/udevd.c
@@ -318,61 +318,42 @@ static void msg_queue_insert(struct udev
 
 static int mem_size_mb(void)
 {
-	int f;
-	char buf[8192];
-	long int len;
-	const char *pos;
-	long int memsize;
-
-	f = open("/proc/meminfo", O_RDONLY);
-	if (f == -1)
-		return -1;
-
-	len = read(f, buf, sizeof(buf)-1);
-	close(f);
+	FILE* f;
+	char buf[1024];
+	long int memsize = -1;
 
-	if (len <= 0)
+	f = fopen("/proc/meminfo", "r");
+	if (f == NULL)
 		return -1;
-	buf[len] = '\0';
 
-	pos = strstr(buf, "MemTotal: ");
-	if (pos == NULL)
-		return -1;
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		long int value;
 
-	if (sscanf(pos, "MemTotal: %ld kB", &memsize) != 1)
-		return -1;
+		if (sscanf(buf, "MemTotal: %ld kB", &value) == 1) {
+			memsize = value / 1024;
+			break;
+		}
+	}
 
-	return memsize / 1024;
+	return memsize;
 }
 
 static int cpu_count(void)
 {
-	int f;
-	char buf[65536];
-	int len;
-	const char *pos;
+	FILE* f;
+	char buf[1024];
 	int count = 0;
 
-	f = open("/proc/stat", O_RDONLY);
-	if (f == -1)
+	f = fopen("/proc/stat", "r");
+	if (f == NULL)
 		return -1;
 
-	len = read(f, buf, sizeof(buf)-1);
-	close(f);
-	if (len <= 0)
-		return -1;
-	buf[len] = '\0';
-
-	pos = strstr(buf, "cpu");
-	if (pos == NULL)
-		return -1;
-
-	while (pos != NULL) {
-		if (strncmp(pos, "cpu", 3) == 0 &&isdigit(pos[3]))
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		if (strncmp(buf, "cpu", 3) == 0 && isdigit(buf[3]))
 			count++;
-		pos = strstr(&pos[3], "cpu");
 	}
 
+	fclose(f);
 	if (count == 0)
 		return -1;
 	return count;
@@ -380,29 +361,24 @@ static int cpu_count(void)
 
 static int running_processes(void)
 {
-	int f;
-	char buf[32768];
-	int len;
-	int running;
-	const char *pos;
+	FILE* f;
+	char buf[1024];
+	int running = -1;
 
-	f = open("/proc/stat", O_RDONLY);
-	if (f == -1)
+	f = fopen("/proc/stat", "r");
+	if (f == NULL)
 		return -1;
 
-	len = read(f, buf, sizeof(buf)-1);
-	close(f);
-	if (len <= 0)
-		return -1;
-	buf[len] = '\0';
-
-	pos = strstr(buf, "procs_running ");
-	if (pos == NULL)
-		return -1;
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		int value;
 
-	if (sscanf(pos, "procs_running %u", &running) != 1)
-		return -1;
+		if (sscanf(buf, "procs_running %u", &value) == 1) {
+			running = value;
+			break;
+		}
+	}
 
+	fclose(f);
 	return running;
 }
 

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

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- 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