Commit 94486dbd authored by Manfred Spraul's avatar Manfred Spraul Committed by Linus Torvalds

[PATCH] fix f_version optimization for get_tgid_list

The kernel contains an optimization that skips the linked list walk in
get_tgid_list for the common case of sequential accesses.  Unfortunately
the optimization is buggy (missing NULL pointer check for the result of
find_task_by_pid) and broken (actually - broken twice: the tgid value that
is stored in f_version is always 0 because tgid is overwritten when the
string is created and additionally the common case is not filldir < 0, it's
running out of nr_tgids).

The attached patch fixes these bugs.

Roger Luethi <rl@hellgate.ch> ran a benchmark:

	test: top -d 0 -b -n 10 > /dev/null

	==> 2.6.8 <==
	real    0m19.092s
	user    0m5.013s
	sys     0m12.622s

	==> 2.6.8 + patch-tgid-bugfixes <==
	real    0m10.062s
	user    0m5.042s
	sys     0m4.111s
Signed-Off-By: default avatarManfred Spraul <manfred@colorfullife.com>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 5c689659
...@@ -1670,7 +1670,7 @@ static int get_tgid_list(int index, unsigned long version, unsigned int *tgids) ...@@ -1670,7 +1670,7 @@ static int get_tgid_list(int index, unsigned long version, unsigned int *tgids)
p = NULL; p = NULL;
if (version) { if (version) {
p = find_task_by_pid(version); p = find_task_by_pid(version);
if (!thread_group_leader(p)) if (p && !thread_group_leader(p))
p = NULL; p = NULL;
} }
...@@ -1733,6 +1733,7 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) ...@@ -1733,6 +1733,7 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
char buf[PROC_NUMBUF]; char buf[PROC_NUMBUF];
unsigned int nr = filp->f_pos - FIRST_PROCESS_ENTRY; unsigned int nr = filp->f_pos - FIRST_PROCESS_ENTRY;
unsigned int nr_tgids, i; unsigned int nr_tgids, i;
int next_tgid;
if (!nr) { if (!nr) {
ino_t ino = fake_ino(0,PROC_TGID_INO); ino_t ino = fake_ino(0,PROC_TGID_INO);
...@@ -1742,26 +1743,45 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) ...@@ -1742,26 +1743,45 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
nr++; nr++;
} }
/* /* f_version caches the tgid value that the last readdir call couldn't
* f_version caches the last tgid which was returned from readdir * return. lseek aka telldir automagically resets f_version to 0.
*/ */
nr_tgids = get_tgid_list(nr, filp->f_version, tgid_array); next_tgid = filp->f_version;
filp->f_version = 0;
for (;;) {
nr_tgids = get_tgid_list(nr, next_tgid, tgid_array);
if (!nr_tgids) {
/* no more entries ! */
break;
}
next_tgid = 0;
for (i = 0; i < nr_tgids; i++) { /* do not use the last found pid, reserve it for next_tgid */
int tgid = tgid_array[i]; if (nr_tgids == PROC_MAXPIDS) {
ino_t ino = fake_ino(tgid,PROC_TGID_INO); nr_tgids--;
unsigned long j = PROC_NUMBUF; next_tgid = tgid_array[nr_tgids];
}
do for (i=0;i<nr_tgids;i++) {
buf[--j] = '0' + (tgid % 10); int tgid = tgid_array[i];
while ((tgid /= 10) != 0); ino_t ino = fake_ino(tgid,PROC_TGID_INO);
unsigned long j = PROC_NUMBUF;
if (filldir(dirent, buf+j, PROC_NUMBUF-j, filp->f_pos, ino, DT_DIR) < 0) { do
filp->f_version = tgid; buf[--j] = '0' + (tgid % 10);
break; while ((tgid /= 10) != 0);
if (filldir(dirent, buf+j, PROC_NUMBUF-j, filp->f_pos, ino, DT_DIR) < 0) {
/* returning this tgid failed, save it as the first
* pid for the next readir call */
filp->f_version = tgid_array[i];
goto out;
}
filp->f_pos++;
nr++;
} }
filp->f_pos++;
} }
out:
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment