Split up deadlkres() to make it more readable in anticipation of
further changes adding another level of indentation.
Some of the logic got simplified with the break out functions.
There should be no functional changes.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| sys/kern/kern_clock.c | ||
|---|---|---|
| 187 ↗ | (On Diff #44106) | You do not need __inline. Compilers are good inlining the code for static functions when needed. |
| 192 ↗ | (On Diff #44106) | I think you should add asserts indicating that allproc_lock, thread' and p' lock are owned. It is the strange, and most important, unneeded weirdness in the interface of these two functions, that you unlock the thread before returning. |
| 207 ↗ | (On Diff #44106) | BTW, why unlock ? |
| 219 ↗ | (On Diff #44106) | Same locking assertion is needed. |
| 237 ↗ | (On Diff #44106) | Perhaps try1 should be bool. |
Comment Actions
For the other comments it's basically the same; I moved the former inline code into sub-functions. I'll make the changes and upload a new patch.
| sys/kern/kern_clock.c | ||
|---|---|---|
| 207 ↗ | (On Diff #44106) | Dunno; the original code did; I basically just moved the former inline code into a function; I am happy to remove these and just panic. |
Comment Actions
Addressed all the comments; not unlocking for panic also
allowed us to remove the tryl variable and just return.
It means we hold the thread lock a bit longer over the string
compare if we check against blessed[]; that's a slow path so
should be fine.
| sys/kern/kern_clock.c | ||
|---|---|---|
| 188 ↗ | (On Diff #44920) | Why do you need '_' in front of the name ? |
Comment Actions
Thanks for the review!
| sys/kern/kern_clock.c | ||
|---|---|---|
| 188 ↗ | (On Diff #44920) | Nope. I guess not. I'll remove them and commit. |