feat(swap-proxy): implement zram dedup storage layer#5
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the swap-proxy backend by decoupling slot allocation from the deduplication table. It introduces a new SlotAllocator to manage backend page slots independently, and updates the TranslationTable to support both direct and deduplicated mapping kinds. The DedupTable has been simplified to focus on metadata tracking and LRU eviction of singly-referenced entries. Feedback on these changes highlights a critical race condition in remove_reference where concurrent operations could lead to double-frees or slot leaks; using DashMap::remove_if is suggested to resolve this atomically.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub fn remove_reference(&self, fp: &Fingerprint) -> ReferenceRemoval { | ||
| let should_remove = if let Some(entry) = self.map.get(fp) { | ||
| let prev = entry.ref_count.fetch_sub(1, Ordering::Relaxed); | ||
| prev <= 1 | ||
| } else { | ||
| false | ||
| return ReferenceRemoval::NotTracked; | ||
| }; | ||
|
|
||
| if should_remove { | ||
| if let Some((_, entry)) = self.map.remove(fp) { | ||
| let slot = entry.backend_offset; | ||
| self.free_slots.push(slot); | ||
|
|
||
| // Remove from LRU | ||
| let mut lru = self.lru.lock(); | ||
| lru.retain(|(f, _)| f != fp); | ||
|
|
||
| return Some(slot); | ||
| return ReferenceRemoval::Removed { | ||
| backend_offset: slot, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| None | ||
| ReferenceRemoval::StillReferenced | ||
| } |
There was a problem hiding this comment.
There is a critical race condition in remove_reference when decremented concurrently with add_reference or evict_lru_metadata. If remove_reference decrements the reference count to 0, another thread can concurrently call add_reference and increment it back to 1 before remove_reference actually removes the entry from the map. This results in the entry being removed from the map anyway, leading to a double-free of the backend slot when both translations are released. Additionally, if evict_lru_metadata removes the entry after it was decremented to 0 but before remove_reference calls remove, remove_reference will return StillReferenced instead of Removed, causing a slot leak.
Using remove_if allows us to atomically decrement and conditionally remove the entry, completely eliminating these race conditions.
pub fn remove_reference(&self, fp: &Fingerprint) -> ReferenceRemoval {
let mut still_referenced = false;
let removed = self.map.remove_if(fp, |_, entry| {
let prev = entry.ref_count.fetch_sub(1, Ordering::Relaxed);
if prev <= 1 {
true
} else {
still_referenced = true;
false
}
});
if let Some((_, entry)) = removed {
let mut lru = self.lru.lock();
lru.retain(|(f, _)| f != fp);
ReferenceRemoval::Removed {
backend_offset: entry.backend_offset,
}
} else if still_referenced {
ReferenceRemoval::StillReferenced
} else {
ReferenceRemoval::NotTracked
}
}There was a problem hiding this comment.
Addressed in c829f1b.
Changes:
remove_referencenow usesDashMap::remove_ifso the refcount decrement and final removal happen under the same shard lock.evict_lru_metadatanow also usesremove_iffor the singly-referenced predicate, avoiding a separate check/remove race while not holding the LRU lock across the DashMap write lock.- Added
test_concurrent_reference_removal_frees_slot_once.
Verification:
cargo test -p animaksm-swap-proxymake check
Add backend slot allocation, explicit direct vs deduplicated translations, safe dedup metadata/refcount ownership, and cleanup on overwrite/discard. Normalize writes to pages, verify duplicate content before sharing backend slots, and fall back to direct mappings when dedup metadata cannot be tracked. This keeps the implementation scoped to the storage/mapping layer; the ublk block-device frontend remains a follow-up.
vi70x4
force-pushed
the
feat/zram-dedup-layer
branch
from
09d8807 to
c829f1b
Compare
2 participants
Summary
Scope
This implements the core storage/mapping layer needed by a zram-dedup block frontend. It does not yet register a real
/dev/ublkb*device or activate swap through ublk; that remains the next patch.Verification
cargo test -p animaksm-swap-proxygit diff --checkcargo fmt --all,make check,make testReady for AI review.