Add support for kernel threads in kvm_getprocs() and the underlying
kvm_proclist() in libkvm when fetching from a kernel core file. This
has been missing/needed for several releases, when kernel threads became
normal threads. Iterating to the next thread or proc uses a goto; I
tried to avoid that, but the result was a larger modification that seemed
more fragile than this version.
Details
Details
- Reviewers
jhb vangyzen cem - Commits
- rS352597: Add support for ps -H on corefiles in libkvm
Tested on amd64 and i386. I couldn't figure out how to generate a
core file on armv6 or aarch64.
Sponsored by: Forcepoint LLC
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Does this fix anything in the base system? I thought 'ps H' worked on vmcores already? Hmm, just tried it on one and indeed it does not.
| lib/libkvm/kvm_proc.c | ||
|---|---|---|
| 142 ↗ | (On Diff #61441) | Maybe make this a while() since it has no pre and post ops? |
| 144 ↗ | (On Diff #61441) | So we re-read the struct proc for each thread. Even though it would mean a lot of indenting, I do wonder if it wouldn't be overall cleaner to structure this as nested loops instead walking over processes and then threads inside the process? You could do the process-common things like p_ucred before the thread loop so that you avoid re-reading them for each thread as well in that case. |
| 587 ↗ | (On Diff #61441) | zombproc was just removed from head, so this might have to get more complicated to deal with that, but that could be a separate change. |
| 611 ↗ | (On Diff #61441) | Maybe at least a comment. We could also have a way to see if kvm_deadprocs ran out of room and if so grow it and try again. |
Comment Actions
Thanks, John.
| lib/libkvm/kvm_proc.c | ||
|---|---|---|
| 142 ↗ | (On Diff #61441) | Sure. |
| 144 ↗ | (On Diff #61441) | Yes, I didn't worry about the performance of ps on a core dump. I did this some time ago (with FreeBSD 8), and was trying to minimize code changes. It's a little odd, as it usually loops over processes but sometimes over both, and both share some termination conditions (buffer full). I'll take another look at that. |
| 587 ↗ | (On Diff #61441) | That certainly makes it easier to MFC. I'll have to look at the zombproc change. |
| lib/libkvm/kvm_proc.c | ||
|---|---|---|
| 144 ↗ | (On Diff #61441) | I started refactoring along these lines. It is very messy; the code to read the substructures and to copy out the bits are interwoven, and share the logic to decide which is valid. I moved the read of the creds, then noticed that it was copying things into the kinfo_proc as it went. Also, it zeros the kinfo_proc at the top of the current loop. If it decides not to include the current entry, it just continues and re-zeros. That makes it harder to preserve the proc and whatever might be common. I'll look a little more, but I'm not convinced this will be a win. |
| lib/libkvm/kvm_proc.c | ||
|---|---|---|
| 144 ↗ | (On Diff #61441) | Ok, if it's a disaster then don't worry about it. The in-kernel code does do it in passes like this (there's a fill_kinfo_proc_only() we call once and then a fill_kinfo_thread()), though there's also some special magic to deal with the INC_THREADS case vs !INC_THREADS (I think the various fill_kinfo_* functions are to deal with that without duplicating lots of code). But libkvm may not warrant that level of complexity. As you note, the perf doesn't matter. |
Comment Actions
I took yet another tack last night, and it is better than previous attempts. It seems to be working correctly, but needs more testing. The changes are much bigger, mostly due to code motions. (svn diff is now 286 lines, about twice as big.) I'll test more, then decide whether to proceed with it.
Comment Actions
Change to use nested loops for procs and threads; deal with zombproc
Rework to use two nested loops; add changes to work with and without
zombproc in the kernel.