From: Manfred Spraul 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 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: Manfred Spraul Signed-off-by: Andrew Morton --- 25-akpm/fs/proc/base.c | 52 +++++++++++++++++++++++++++++++++---------------- 1 files changed, 36 insertions(+), 16 deletions(-) diff -puN fs/proc/base.c~fix-f_version-optimization-for-get_tgid_list fs/proc/base.c --- 25/fs/proc/base.c~fix-f_version-optimization-for-get_tgid_list 2004-09-02 21:05:19.602220928 -0700 +++ 25-akpm/fs/proc/base.c 2004-09-02 21:05:19.608220016 -0700 @@ -1693,7 +1693,7 @@ static int get_tgid_list(int index, unsi p = NULL; if (version) { p = find_task_by_pid(version); - if (!thread_group_leader(p)) + if (p && !thread_group_leader(p)) p = NULL; } @@ -1756,6 +1756,7 @@ int proc_pid_readdir(struct file * filp, char buf[PROC_NUMBUF]; unsigned int nr = filp->f_pos - FIRST_PROCESS_ENTRY; unsigned int nr_tgids, i; + int next_tgid; if (!nr) { ino_t ino = fake_ino(0,PROC_TGID_INO); @@ -1765,26 +1766,45 @@ int proc_pid_readdir(struct file * filp, nr++; } - /* - * f_version caches the last tgid which was returned from readdir + /* f_version caches the tgid value that the last readdir call couldn't + * return. lseek aka telldir automagically resets f_version to 0. */ - nr_tgids = get_tgid_list(nr, filp->f_version, tgid_array); - - for (i = 0; i < nr_tgids; i++) { - int tgid = tgid_array[i]; - ino_t ino = fake_ino(tgid,PROC_TGID_INO); - unsigned long j = PROC_NUMBUF; + 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; - do - buf[--j] = '0' + (tgid % 10); - while ((tgid /= 10) != 0); + /* do not use the last found pid, reserve it for next_tgid */ + if (nr_tgids == PROC_MAXPIDS) { + nr_tgids--; + next_tgid = tgid_array[nr_tgids]; + } - if (filldir(dirent, buf+j, PROC_NUMBUF-j, filp->f_pos, ino, DT_DIR) < 0) { - filp->f_version = tgid; - break; + for (i=0;if_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; } _