Sponsored by: Rubicon Communications, LLC ("Netgate")
Details
- Reviewers
kib markj - Group Reviewers
pfsense - Commits
- rG928864a93594: fix build with LOCK_PROFILING but without KDTRACE_HOOKS
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 60864 Build 57748: arc lint + arc unit
Event Timeline
| sys/kern/kern_rwlock.c | ||
|---|---|---|
| 476 | I do not understand this block. LOCK_PROFILING does not depend on KDTRACE_HOOK. While your patch puts LOCK_PROFILING control inside KDTRACE_HOOK block. | |
| sys/kern/kern_rwlock.c | ||
|---|---|---|
| 476 | I think I misunderstood the intent behind doing_lockprof. It's to allow lock profiling to be enabled either unconditionally (with LOCK_PROFILING) or conditionally. (if KDTRACE_HOOKS and LOCKSTAT_PROFILE_ENABLED() returns true). So the correct thing here would be: #if defined(KDTRACE_HOOKS) uintptr_t state = 0; #endif #if defined(KDTRACE_HOOKS) || defined(LOCK_PROFILING) int doing_lockprof = 0; #endif (And then not have this #ifdef LOCK_PROFILING here, of course) | |
| sys/kern/kern_rwlock.c | ||
|---|---|---|
| 476 | Yes, this sounds more correct. | |
Review remarks
We now unconditionally assign 'state = v' (ifdef KDTRACE_HOOKS), but that should
be fine, because we only ever use 'state' if doing_lockprof is true, which is
when it was assigned prior to this change.
Updating D47822: fix build with LOCK_PROFILING but without KDTRACE_HOOKS
| sys/kern/kern_rwlock.c | ||
|---|---|---|
| 478 | I don't see how this assignment can be removed. Without we'd always have state = 0. | |
| sys/kern/kern_rwlock.c | ||
|---|---|---|
| 478 | It’ll be set if doing_lockprof == 1, which is the only case we actually use it. | |
| sys/kern/kern_rwlock.c | ||
|---|---|---|
| 478 | I think I misunderstood kib's comment because it applied to an old version of the file, so the comment appears in a misleading place. | |