Record spinlock holder identity for datafile/journal deadlock fatals - #22725
Merged
stelfrag merged 2 commits intoJun 16, 2026
Conversation
Replaced SPINLOCK with SPINLOCK_TRACKED in critical database and journalfile code paths to enable deadlock detection and holder identification. Updated accompanying locking logic and introduced tracked spinlock helpers in `spinlock.h`.
Contributor
There was a problem hiding this comment.
No issues found across 7 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant DC as Deadlock Checker (spinlock_core)
participant ST as SPINLOCK_TRACKED struct
participant DF as Datafile (users.spinlock)
participant JF as Journalfile (data_spinlock)
participant FL as Flush Thread (rrdengine.c)
Note over DC,FL: Tracked spinlock flow on contention
DF->>ST: spinlock_tracked_lock()
activate ST
ST->>DC: spinlock_lock_core (tracked=non-NULL)
activate DC
loop Spin-acquire loop
DC->>DC: Test __atomic_try_lock
alt Lock acquired
DC->>ST: spinlock_tracked_record_holder()
ST->>ST: __atomic_store holder_tid, holder_func, holder_since_ut
DC-->>DF: lock owned
DF->>DF: (critical section: acquire/release datafile refs)
DF->>ST: spinlock_tracked_unlock()
ST->>ST: __atomic_clear holder_tid
ST->>DC: spinlock_unlock (plain)
deactivate DC
deactivate ST
else Lock busy (contention)
DC->>DC: spins++
alt Every SPINS_BEFORE_DEADLOCK_CHECK iteration
DC->>ST: spinlock_tracked_deadlock_detect()
activate ST
alt Timeout exceeded (3600s)
DC->>ST: __atomic_load holder_tid, holder_func, holder_since_ut
alt holder_tid != 0
ST-->>DC: holder identity known
DC->>DC: fatal("... holder: tid=%d func='%s' held_for=%"PRIi64"s ...")
else holder_tid == 0 (stuck/corrupted)
ST-->>DC: no holder recorded
DC->>DC: fatal("... holder: NONE - possible corruption/stuck byte ...")
end
else No timeout yet
ST-->>DC: continue waiting
end
deactivate ST
end
DC->>DC: microsleep (exponential backoff)
end
end
Note over JF,DC: Journalfile parallel example (same pattern)
JF->>ST: spinlock_tracked_lock()
ST->>DC: spinlock_lock_core (tracked=non-NULL)
activate DC
DC-->>JF: lock owned
JF->>JF: (critical section: mount/unmount v2 data, update metrics)
JF->>ST: spinlock_tracked_unlock()
deactivate DC
Note over FL,DC: Flush-thread update via tracked lock
FL->>ST: spinlock_tracked_lock()
ST->>DC: spinlock_lock_core (tracked=non-NULL)
activate DC
DC-->>FL: lock owned
FL->>FL: update last_time_s on journalfile
FL->>ST: spinlock_tracked_unlock()
deactivate DC
Note over ST: Plain SPINLOCK path via spinlock_lock_with_trace<br/>passes tracked=NULL, constant-folded away<br/>— no overhead, no holder stores
- Updated SPINLOCK_TRACKED to retain holder fields after unlock for consistent deadlock diagnostics. - Added mutex-backed SPINLOCK_TRACKED implementation to align functionality across spinlock types. - Refactored and clarified comments to explain holder field handling and deadlock detection behavior.
|
Collaborator
Author
|
@cubic-dev-ai review PR |
Contributor
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
No issues found across 7 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant DF as datafile.c (users.*)
participant JF as journalfile.c (data_spinlock)
participant RE as rrdengine.c (extent_flush)
participant SL as SPINLOCK (plain)
participant ST as SPINLOCK_TRACKED
participant DET as Deadlock Detector
Note over DF,RE: NEW: SPINLOCK_TRACKED replaces SPINLOCK for these locks
DF->>ST: spinlock_tracked_lock(&df->users.spinlock)
ST->>SL: spinlock_lock_core(spinlock, func, tracked)
alt Lock contested (spinning)
loop Exponential backoff + deadlock check
ST->>ST: spins++
alt spins % SPINS_BEFORE_DEADLOCK_CHECK == 0
ST->>DET: spinlock_tracked_deadlock_detect()
DET->>DET: Read holder_tid, holder_func, holder_since_ut
alt held >= SPINLOCK_DEADLOCK_TIMEOUT_SEC
DET-->>ST: fatal("DEADLOCK DETECTED [holder: tid=N func='...' held_for=N]")
end
end
ST->>ST: microsleep(usec), usec *= 2
end
end
ST->>ST: Acquired: __atomic_add_fetch(&locked, 1)
ST->>ST: spinlock_tracked_record_holder()
ST-->>DF: lock acquired
DF->>DF: Critical section (check users.available, lockers, etc.)
DF->>ST: spinlock_tracked_unlock(&df->users.spinlock)
ST->>SL: spinlock_unlock(spinlock)
Note over ST: Holder fields NOT cleared (advisory - overwritten by next acquirer)
ST-->>DF: lock released
Note over DF,RE: Same pattern for all call sites in datafile.c/journalfile.c/rrdengine.c
JF->>ST: spinlock_tracked_lock(&journalfile->data_spinlock)
ST->>SL: spinlock_lock_core(spinlock, func, tracked)
alt Contested
ST->>DET: spinlock_tracked_deadlock_detect()
DET-->>ST: Holder identity in fatal
end
ST->>ST: Record holder
ST-->>JF: lock acquired
JF->>JF: Critical section (check JOURNALFILE flags, refcount, mmap)
JF->>ST: spinlock_tracked_unlock(&journalfile->data_spinlock)
ST-->>JF: lock released
RE->>ST: spinlock_tracked_lock(&datafile->journalfile->data_spinlock)
RE->>RE: Update last_time_s
RE->>ST: spinlock_tracked_unlock(&datafile->journalfile->data_spinlock)
alt SPINLOCK_IMPL_WITH_MUTEX build
Note over ST: Mutex-backed implementation (no spin loop)
ST->>SL: spinlock_lock(&spinlock->spinlock) (calls netdata_mutex_lock)
ST->>ST: spinlock_tracked_record_holder()
ST-->>DF: lock acquired
DF->>ST: spinlock_tracked_unlock()
ST->>SL: spinlock_unlock(&spinlock->spinlock) (calls netdata_mutex_unlock)
Note over ST: holder fields recorded but no deadlock detection
end
thiagoftsm
approved these changes
Jun 15, 2026
thiagoftsm
left a comment
Contributor
There was a problem hiding this comment.
No issues found during runtime after hours running. LGTM!
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
…22725) * Switch to SPINLOCK_TRACKED for enhanced deadlock diagnostics Replaced SPINLOCK with SPINLOCK_TRACKED in critical database and journalfile code paths to enable deadlock detection and holder identification. Updated accompanying locking logic and introduced tracked spinlock helpers in `spinlock.h`. * Improve SPINLOCK_TRACKED behavior and add mutex-backed implementation - Updated SPINLOCK_TRACKED to retain holder fields after unlock for consistent deadlock diagnostics. - Added mutex-backed SPINLOCK_TRACKED implementation to align functionality across spinlock types. - Refactored and clarified comments to explain holder field handling and deadlock detection behavior. (cherry picked from commit 0daffba)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spinlock.h.Summary by cubic
Add holder-aware spinlocks to improve deadlock diagnostics in the DB engine. Datafile and journal locks now report the blocking thread (tid, function, held-for), and tracked info persists across unlock; works under the mutex-backed build too.
New Features
SPINLOCK_TRACKEDwith holder tracking andspinlock_tracked_*APIs; holder fields persist after unlock for accurate reporting.SPINLOCKcost unchanged.SPINLOCK_TRACKEDso holder recording works whenSPINLOCK_IMPL_WITH_MUTEXis enabled.Refactors
users.spinlockand journaldata_spinlocktoSPINLOCK_TRACKED, updated call sites indatafile.c,journalfile.c, andrrdengine.c, and fixed thespinlock_initmacro typo inspinlock.h.Written for commit 59055de. Summary will update on new commits.