Hi Matthias, thanks for reply. Would you please wraparound the text when writing a reply? Each line becomes way too long on text based editors :-)
Anyway, moving on to the topic... On Thu, Sep 10, 2026 at 09:46:40PM +0800, Matthias Goergens wrote: > Thanks. I ran into this while testing bcachefs performance changes. > The bcachefs ktest end checks write `do_rcu_barrier` before reading > `/proc/allocinfo`, with the expectation that allocations still > reported afterwards are leaks. Small objects released with > `kfree_rcu()` remained visible after repeated writes to the hook and > 20 seconds of waiting, so otherwise clean tests failed their leak > check. Thanks. Some background like this would be nice to be covered in the cover letter or commit message :-) > Strictly, that ktest is assuming a stronger contract than the hook > currently documents: `do_rcu_barrier` promises an ordinary > `rcu_barrier()`, not a complete drain of objects still held in > `kfree_rcu()` batching. I nevertheless think the stronger behaviour > is useful for this test-only quiescence hook, because it lets > allocation-leak checks reliably separate deferred frees from genuine > leaks. Ack. > I followed those allocations across repeated filesystem lifecycles. > Their number eventually fell when an RCU sheaf filled, so I have no > evidence that this path grows without bound or causes OOM. Ah, the reason I mentioned unbounded amount of objects was because the commit 16128b1f8c823438dc that introduced the knob explains what can go wrong (e.g OOMs during the test) without the rcutree.do_rcu_barrier. > The problem I observed is limited to test isolation: the hook can > leave deferred frees behind and make them look like leaks. With the > proposed change, the same unmodified bcachefs workload passed the > leak check. It'd be more convincing if this part is included in the changelog ;-) > I also found that this exact follow-on was discussed when > `kvfree_rcu_barrier()` was added in 2024: Paul proposed calling it > from `rcu_barrier_throttled()` for clean userspace benchmark > baselines, and Uladzislau agreed that adding it and documenting both > operations was safest: > https://lore.kernel.org/all/[email protected]/ Didn't notice that, and makes sense given the purpose of the knob. > I will omit the `Fixes:` tag in v2 and present this as a strengthening > of the test interface. Makes sense to me. -- Cheers, Harry / Hyeonggon
