This can be disabled by sysctl kern.core_dump_can_intr
Tested by: pho
Details
Details
- Reviewers
markj imp - Commits
- rGb5cadc643e85: Make core dump writes interruptible with SIGKILL
rG2ec505d2b863: core(5): explicitly mention the core file size limit name
rG244ab56611c1: Add curproc_sigkilled()
rG7a9775e9d06f: sys/proc.h: remove dead prototype
rGdc2d0899bba6: kern_sig.c: Remove unused SIGPROP_CANTMASK
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
It would be good to document this behaviour in core.5, though it is logically not quite the right place. Or at least add a release note.
| sys/kern/kern_exec.c | ||
|---|---|---|
| 1956 ↗ | (On Diff #96264) | At this point we will needlessly call vn_truncate_locked() before breaking out of the outer loop. Why not break out immediately? |
| sys/kern/kern_sig.c | ||
| 3300 | Maybe curproc_has_sigkill_pending()? I think this name is not very clear. | |
| 3308 | Perhaps atomic_load, since there is no synchronization. | |
| share/man/man5/core.5 | ||
|---|---|---|
| 56 ↗ | (On Diff #96273) | limited by the |
| 59 ↗ | (On Diff #96273) | |
| 60 ↗ | (On Diff #96273) | |
| 62 ↗ | (On Diff #96273) | |
| 64 ↗ | (On Diff #96273) | Or s/later/latter/, but I think this would be an unusual use of "latter". |
| sys/kern/kern_exec.c | ||
| 155 ↗ | (On Diff #96273) | |
| 1954 ↗ | (On Diff #96273) | No need to set success. |
| sys/kern/kern_sig.c | ||
|---|---|---|
| 3300 | I'm not sure what "9" is supposed to mean, or why it is preferable to spelling out "sigkill". | |
Comment Actions
Sorry, I forgot to submit two answers to the notes in previous patch update.
| sys/kern/kern_sig.c | ||
|---|---|---|
| 3300 | Pending usually means that signal was not delivered yet due to userspace blocking or something similar. SIGKILL cannot be pending (at least from userspace PoV). I changed it to curproc_is_killed_9, it should be vivid enough. | |
| 3300 | 9 is SIGKILL, but ok | |
| 3308 | We do unlocked read this way everywhere. Also I do not think that the load through volatile actually improves anything there. | |
Comment Actions
Man page fixes
RWTUN
remove setting of success
rename function to curproc_sigkilled()
| share/man/man5/core.5 | ||
|---|---|---|
| 60 ↗ | (On Diff #96341) | s/With a large enough limit, processes that had/With a large limit, a process that had/ I think reads better and conveys the same meaning. And it isn't processes, in aggregate, that causes a problem. It's the mapped address space of a single process that's a problem when that process dumps core. |
| 63 ↗ | (On Diff #96341) | I think this sentence is awkward. "The system ignores all signals sent to a process writing a core file, except |
| 68 ↗ | (On Diff #96341) | What's the normal way to indicate that this sysctl is also a tunable? |
| sys/kern/kern_exec.c | ||
| 154 ↗ | (On Diff #96341) | "intr" seems too concise here. I'd be tempted to call the core_dump_can_intr or core_dump_interrupible (with a preference for the former) mostly because I can read core_dump_intr as some async signal to interrupt the core dump writing. |
| 1943 ↗ | (On Diff #96341) | I'd move interrupted up a line. It doesn't control the loop at all and just needs to be initialized. |
| 1954 ↗ | (On Diff #96341) | What does setting interrupted and breaking a couple of times offer over a 'return (EINTR)' here? |