Skip to content

Commit 44be98b

Browse files
genevievehelselmeta-codesync[bot]
authored andcommitted
indexedlog: check change detector mappings under BUFFERS contention
Summary: Fixes `page_out::find_region()` so failure to lock the regular mmap `page_out::BUFFERS` does not prevent checking the `change_detect::BUFFERS`. This lets the SIGBUS handler still find and recover `rlock` change detector mmaps when the main buffer lock is contended. Also updates the regression test from the previous diff by removing the `FIXME` and flipping the expected result from `None` to `Some(true)`. Reviewed By: quark-zju Differential Revision: D106845173 fbshipit-source-id: 38d08a22bdc08d68c9c113d88e24b5d78bcf4e0f
1 parent 8c82384 commit 44be98b

1 file changed

Lines changed: 15 additions & 11 deletions

File tree

eden/scm/lib/indexedlog/src/page_out.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,20 @@ pub(crate) fn track_mmap_buffer(bytes: &Bytes) {
8383
/// Does not block. Returns `None` when unable to take the lock.
8484
#[cfg(unix)]
8585
pub(crate) fn find_region(addr: usize) -> Option<(usize, usize, bool)> {
86-
let locked = BUFFERS.try_lock().ok()?;
87-
if let Some((start, end)) = locked.find_region(addr) {
88-
return Some((start, end, false));
86+
if let Ok(locked) = BUFFERS.try_lock() {
87+
if let Some((start, end)) = locked.find_region(addr) {
88+
return Some((start, end, false));
89+
}
8990
}
91+
9092
// Also check the change_detect mmap buffers.
91-
let locked = crate::change_detect::BUFFERS.try_lock().ok()?;
92-
locked
93-
.find_region(addr)
94-
.map(|(start, end)| (start, end, true))
93+
if let Ok(locked) = crate::change_detect::BUFFERS.try_lock() {
94+
if let Some((start, end)) = locked.find_region(addr) {
95+
return Some((start, end, true));
96+
}
97+
}
98+
99+
None
95100
}
96101

97102
impl<W: WeakSlice> WeakBuffers<W> {
@@ -198,12 +203,11 @@ mod tests {
198203
let _detector = SharedChangeDetector::new(mmap);
199204

200205
let _log_buffers = super::BUFFERS.lock().unwrap();
201-
// FIXME: find_region should still check change_detect::BUFFERS when
202-
// BUFFERS is locked so the SIGBUS handler can recover rlock mmaps. Flip
203-
// this to Some(true) when the lock-contention bug is fixed.
206+
// Holding BUFFERS must not prevent the SIGBUS handler from finding
207+
// rlock mmaps tracked by change_detect::BUFFERS.
204208
assert_eq!(
205209
super::find_region(addr).map(|(_start, _end, writable)| writable),
206-
None
210+
Some(true)
207211
);
208212
}
209213
}

0 commit comments

Comments
 (0)