fix f_version optimization for get_tgid_list
PATCH 1/1 fix f_version optimization for get_tgid_list
Hi,
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: could you run your 100k processes test against a kernel with this fix applied? I'm just interested how much it helps. One obvious result are fewer getdents64 syscalls: instead of returning 20 entries (480 bytes) the new code fills the user space buffer completely (42 entries/syscall).
Andrew, could you add the patch to your -mm tree?
fs/proc/base.c +37 -17
@@ -1689,7 +1689,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 ; }
@@ -1752,6 +1752,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 );
@@ -1761,26 +1762,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 ; - -do - buf[--j] ='0' + (tgid %10 ); -while ((tgid /=10 ) !=0 ); - -if (filldir (dirent, buf+j,PROC_NUMBUF -j, filp->f_pos, ino,DT_DIR ) <0 ) { - filp->f_version = tgid; + 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 ; } - filp->f_pos++; + next_tgid =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]; + } + +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 ; + +do + buf[--j] ='0' + (tgid %10 ); +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++; + } } +out:return 0 ; }
test: top -d 0 -b -n 10 > /dev/null
==> 2.6.8 <==
real 0m19.092s user 0m5.013s sys 0m12.622s
CPU: CPU with timer interrupt, speed 0 MHz (estimated) Profiling through timer interrupt
samples % image name symbol name 6338 35.9379 vmlinux get_tgid_list 1742 9.8775 vmlinux pid_alive 1264 7.1672 libc-2.3.3.so _IO_vfscanf_internal 940 5.3300 vmlinux number 625 3.5439 vmlinux proc_pid_stat 532 3.0166 libc-2.3.3.so _IO_vfprintf_internal 398 2.2567 vmlinux __d_lookup 297 1.6841 vmlinux vsnprintf 268 1.5196 vmlinux link_path_walk 266 1.5083 libc-2.3.3.so __i686.get_pc_thunk.bx 214 1.2134 libc-2.3.3.so ____strtoul_l_internal 210 1.1907 vmlinux task_statm 209 1.1851 libc-2.3.3.so _IO_default_xsputn_internal 199 1.1284 libc-2.3.3.so _IO_putc_internal 172 0.9753 libc-2.3.3.so ____strtol_l_internal 135 0.7655 libc-2.3.3.so _IO_sputbackc_internal 130 0.7371 libncurses.so.5.4 _nc_outch 124 0.7031 vmlinux pid_revalidate 115 0.6521 top task_show 112 0.6351 libc-2.3.3.so __find_specmb 103 0.5840 vmlinux atomic_dec_and_lock 98 0.5557 libc-2.3.3.so __funlockfile 76 0.4309 libncurses.so.5.4 tputs 73 0.4139 vmlinux system_call 72 0.4083 libc-2.3.3.so _IO_str_overflow_internal 72 0.4083 libc-2.3.3.so __GI___printf_fp 66 0.3742 vmlinux may_open 65 0.3686 libc-2.3.3.so __GI_strrchr 61 0.3459 vmlinux path_lookup 60 0.3402 libc-2.3.3.so __GI___errno_location 58 0.3289 libc-2.3.3.so __flockfile 55 0.3119 vmlinux __might_sleep
==> 2.6.8 + patch-tgid-bugfixes <==
real 0m10.062s user 0m5.042s sys 0m4.111s
CPU: CPU with timer interrupt, speed 0 MHz (estimated) Profiling through timer interrupt
samples % image name symbol name 1330 14.5292 libc-2.3.3.so _IO_vfscanf_internal 931 10.1704 vmlinux number 618 6.7511 vmlinux proc_pid_stat 494 5.3965 libc-2.3.3.so _IO_vfprintf_internal 307 3.3537 vmlinux __d_lookup 296 3.2336 vmlinux vsnprintf 233 2.5453 libc-2.3.3.so _IO_putc_internal 216 2.3596 libc-2.3.3.so _IO_default_xsputn_internal 212 2.3159 libc-2.3.3.so ____strtoul_l_internal 208 2.2722 libc-2.3.3.so __i686.get_pc_thunk.bx 200 2.1848 libc-2.3.3.so ____strtol_l_internal 199 2.1739 vmlinux link_path_walk 198 2.1630 vmlinux task_statm 156 1.7042 libc-2.3.3.so _IO_sputbackc_internal 128 1.3983 libncurses.so.5.4 _nc_outch 122 1.3328 top task_show 110 1.2017 vmlinux pid_revalidate 84 0.9176 libc-2.3.3.so __funlockfile 81 0.8849 libc-2.3.3.so __find_specmb 72 0.7865 libncurses.so.5.4 tputs 68 0.7428 vmlinux system_call 65 0.7101 libc-2.3.3.so __GI___errno_location 64 0.6991 libc-2.3.3.so _IO_str_overflow_internal 63 0.6882 libc-2.3.3.so __flockfile 63 0.6882 vmlinux get_tgid_list 61 0.6664 libc-2.3.3.so __strnlen 59 0.6445 libc-2.3.3.so __GI___printf_fp 57 0.6227 vmlinux atomic_dec_and_lock 55 0.6008 libc-2.3.3.so __GI_strrchr 54 0.5899 vmlinux may_open 52 0.5681 libproc-3.2.3.so escape_str 51 0.5571 libc-2.3.3.so _IO_str_init_static_internal 51 0.5571 libc-2.3.3.so __GI___strtoul_internal