On Thu, Sep 3, 2026 at 5:43 PM, Mykyta Yatsenko wrote: > I'm not sure if this change fixes anything, the main issue of walking > concurrently modified rhashtable is not changed: you still may miss > elements or visit same elements multiple times. > > In some scenarios this can make things worse: imagine you start > iterating with small map (visit_budget = 10), then 1000000 elements > are inserted concurrently with walk, so you'll miss at least > 1000000 - 10. > > To me this is a trade off/taste thing, rather than bug fix. The > change is compact, though, I'm not against it.
Thanks for the review. I agree that this change does not, and is not intended to, make concurrent rhashtable iteration complete or duplicate-free. Missed and duplicate elements remain part of the existing best-effort semantics. The narrower issue I am trying to address is that bpf_each_rhash_elem() currently keeps calling rhashtable_next_key() until it returns NULL. The rhashtable_next_key() documentation states that a full iteration may not terminate under adversarial or sustained rehashing, and recommends that callers bound such walks externally. The purpose of this change is therefore not to make the iteration complete, but to provide a finite upper bound on callback invocations under sustained rehashing. I agree that taking a snapshot of ht.nelems makes the bound insensitive to elements inserted after the walk starts. In the example you gave, the walk could stop after the initial occupancy and miss most newly inserted elements. Using map->max_entries would preserve more of the existing behavior under concurrent growth, at the cost of allowing a much longer walk for a sparsely populated map. Would map->max_entries be a more appropriate bound in your view? Also, if you consider adding such a bound useful but not a bug fix, would bpf-next be a more appropriate target for this change? Thanks, Hui
