Add kallsyms_lookup() result cache
Quoted 10 lines · Andi Kleen
Quoted 3 lines · Brent Casavant
On 2.6 based systems, the top command utilizes /proc/[pid]/wchan to determine WCHAN symbol name information. This information is provided by the kernel function
kallsyms_lookup(), which expands a stem-compressedThat sounds more like a bug in your top to me.
/proc/*/wchanitself does not access kallsyms, it just outputs a number.My top doesn't do that.
To get a working WCHAN column on a 2.6.xx kernel, your top must do that. You also must have CONFIG_KALLSYMS enabled.
Quoted 7 lines
Are you saying your top reads
/proc/kallsymson each redisplay? That sounds completely wrong - it should only read the file once and cache it and then look the numbers it is reading from wchan in the cache.Doing the cache in the kernel is the wrong place. This should be fixed in user space.
No way, because:
1. kernel modules may be loaded or unloaded at any time
2. the /proc/*/wchan files don't provide both name and address
I'd be happy to make top (and the rest of procps) use a cache if those problems were addressed. I need a signal sent on module load/unload, or a real /proc/kallsyms st_mtime that I can poll. I also need to have the numeric wchan address in the /proc/*/wchan file, such that it is reliably the same thing as the function name already found there.
Quoted 6 lines · Jesse Barnes
/proc/<pid>/wchan reports the function name as a string. You're arguing that doing that in the kernel is the wrong thing to do, right? If so, it would make sense to change its behavior. Either way, I guess we can fix top to use /proc/<pid>/stat instead, and lookup symbols in an external System.map file.
This is exactly what top does when /proc/*/wchan is missing. It does not work well, due to kernel modules, and also due to the inability to reliably determine if a System.map file matches the currently running kernel. Also, simply parsing that file is slow due to the size of it.
Quoted 4 lines · Brent Casavant also
The cache size of 32 items was chosen experimentally. 16 was too few to cache all the results on even a small system with approximatley 100 tasks, and 64 was too large for all but the most extreme cases.
I found that you need about 64 when using a simple hash, but going to 256 is really cheap so why not? The old procps symbol cache code (now unused) worked well. I notice that you are scanning an array for ranges; it is better to hash on exact address matches. Also, failures get cached and no attempt was made to keep multiple items in a hash bucket.
Here it is. The rest of the code should be obvious.
typedef struct symb {
const char *name;
unsigned KLONG addr;
} symb;
...
static const symb fail = { "?", 0 };
...
static symb hashtable[256];
...
hash = (address >> 4) & 0xff;
Quoted 4 lines
Further work obviously needs to be done for top itself to reduce CPU utilization even further, but this is a large first step. Some quick experiments indicate that the slow path has now moved to other areas of code, which will be addressed in due course.
Jim and I have already used profilers on top. He mostly used gprof. I mostly used gcc's ability to call some custom code on function entry and exit, counting either the function calls or the CPU cycles they consumed. So, lots of luck to you, but...
(eh, this assumes you're using the all-new top in procps 3.x.x)
If you want things fast, provide the DWARF2 debug info needed to make use of /dev/mem. >:-) The LKCD project might have a patch for this; they were converting from STABS to DWARF2 last I heard.
Quoted 4 lines · Brent Casavant later
Hmm. procps version 3.1.3 introduced the use of
/proc/*/wchanwhere possible. I'm using 2.0.13 normally, which appears to have the same behavior (can't find a changelong for the 2 series at the moment).
Oh. Well. There's your problem. The procps-3.x.x code will only read /proc/*/wchan for the processes shown on your screen. It also doesn't corrupt wchan with a buffer overflow.
Quoted 6 lines · Albert Cahalan
Quoted 6 lines
Are you saying your top reads
/proc/kallsymson each redisplay? That sounds completely wrong - it should only read the file once and cache it and then look the numbers it is reading from wchan in the cache.
Quoted 6 lines
Quoted 2 lines
Doing the cache in the kernel is the wrong place. This should be fixed in user space.
No way, because:
1. kernel modules may be loaded or unloaded at any time
We seem to have coped alright under 2.4.
Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txtQuoted 7 lines · Herbert Xu
Quoted 6 lines · Albert Cahalan
Quoted 6 lines
Are you saying your top reads
/proc/kallsymson each redisplay? That sounds completely wrong - it should only read the file once and cache it and then look the numbers it is reading from wchan in the cache.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Doing the cache in the kernel is the wrong place. This should be fixed in user space.
No way, because:
1. kernel modules may be loaded or unloaded at any time
We seem to have coped alright under 2.4.
Not really. We coped alright long ago. Things were getting worse year by year, with modules becoming more popular and their symbols not 100% exported.
Quoted 7 lines · Albert Cahalan
Quoted 3 lines
Doing the cache in the kernel is the wrong place. This should be fixed in user space.
No way, because:
1. kernel modules may be loaded or unloaded at any time
Poll /proc/modules?
Quoted 1 line
2. the
/proc/*/wchanfiles don't provide both name and address
/proc/stat has the address, /proc/kallsyms gives the symbol?
Quoted 6 lines
I'd be happy to make top (and the rest of procps) use a cache if those problems were addressed. I need a signal sent on module load/unload, or a real
/proc/kallsymsst_mtimethat I can poll. I also need to have the numeric wchan address in the/proc/*/wchanfile, such that it is reliably the same thing as the function name already found there.
Updating mtime on /proc/modules may be more logical, or even both.
Or put a modprobe sequence number somewhere in /proc and look for changes in that.
It definitely needs to be fixed, but it doesn't seem that adding code to the kernel is needed.
Quoted 10 lines · Andrew Morton
Quoted 6 lines · Albert Cahalan
Quoted 2 lines
Doing the cache in the kernel is the wrong place.
This should be fixed in user space.No way, because:
1. kernel modules may be loaded or unloaded at any time
Poll
/proc/modules?
Ugh. I like the sequence number suggestion better. This is about speeding things up after all.
A signal that I could register to get would be best of all.
Quoted 3 lines
Quoted 1 line
2. the
/proc/*/wchanfiles don't provide both name and address
/proc/stathas the address,/proc/kallsymsgives the symbol?
Eh, why did we add the /proc/*/wchan files then if not to use them?
One problem with parsing /proc/kallsyms is that I can't mmap it and do a binary search. (the System.map was done this way) It looks like I'd have to read half a megabyte of text, then sort it for later use. It's highly desirable that the procps tools not eat lots of memory or suffer high start-up costs.
Quoted 7 lines
Quoted 6 lines
I'd be happy to make top (and the rest of procps) use a cache if those problems were addressed. I need a signal sent on module load/unload, or a real
/proc/kallsymsst_mtimethat I can poll. I also need to have the numeric wchan address in the/proc/*/wchanfile, such that it is reliably the same thing as the function name already found there.
Quoted 7 lines
Updating mtime on
/proc/modulesmay be more logical, or even both.Or put a modprobe sequence number somewhere in /proc and look for changes in that.
It definitely needs to be fixed, but it doesn't seem that adding code to the kernel is needed.
I'm not so sure anything needs to be fixed, save for SGI upgrading to a more modern procps. There are many more important things:
Supply a real SUSv3-compliant %CPU
Supply whole-process CPU usage data
Add a /proc/*/tty symlink
Allocated PIDs by least-recently-used for safety
Add an "adopted child" flag (for processes reparented to init) Supply the sleep time (in seconds for example)
Supply jobc, the "count of processes qualifying PGID for job control" Supply the raw task_struct and the needed debug info
Quoted 2 lines · Albert Cahalan
I'm not so sure anything needs to be fixed, save for SGI upgrading to a more modern procps. There are many more important things:
OK, I looked into this more closely, and gave procps 3.2.1 a spin. This gives us a similar speedup (top now only consumes 60% of a CPU) to that which I obtained by cacheing symbol lookups and using an old procps. This difference is certainly explainable by top now only obtaining wchan information for displayed processes.
60% is far better than 800%, so this is certainly progress. However 60% is also still quite a bit of CPU time. I'll spend some cycles trying to whittle it down some more, but I'm not all that hopeful.
Thanks for the discussion, it was certainly enlightening.
Brent
Signature · 3 lines
Brent Casavant [email protected] Forget bright-eyed and Operating System Engineer http://www.sgi.com/ bushy-tailed; I'm red- Silicon Graphics, Inc. 44.8562N 93.1355W 860F eyed and bushy-haired.
Quoted 10 lines · Brent Casavant
Quoted 2 lines · Albert Cahalan
I'm not so sure anything needs to be fixed, save for SGI upgrading to a more modern procps. There are many more important things:
OK, I looked into this more closely, and gave procps 3.2.1 a spin. This gives us a similar speedup (top now only consumes 60% of a CPU) to that which I obtained by cacheing symbol lookups and using an old procps. This difference is certainly explainable by top now only obtaining wchan information for displayed processes.
Quoted 3 lines
60% is far better than 800%, so this is certainly progress. However 60% is also still quite a bit of CPU time. I'll spend some cycles trying to whittle it down some more, but I'm not all that hopeful.
Is the 60% mainly kernel or user time now? There were some good ideas in this thread that we could use to speed up symbol table searching. And if opening and reading /proc/kallsyms is taking a significant portion of the time, we could probably trim it down too by only reading it once and polling for module insertion/removal.
Jesse