Hi Mathieu, On Sun, Dec 18, 2022 at 3:56 PM Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote: > > On 2022-12-18 14:13, Joel Fernandes (Google) wrote: > > Hello, I believe the pre-flip memory barrier is not required. The only reason I > > can say to remove it, other than the possibility that it is unnecessary, is to > > not have extra code that does not help. However, since we are issuing a fully > > memory-barrier after the flip, I cannot say that it hurts to do it anyway. > > > > For this reason, please consider these patches as "informational", than a > > "please merge". :-) Though, feel free to consider merging if you agree! > > > > All SRCU scenarios pass with these, with 6 hours of testing. > > Hi Joel, > > Please have a look at the comments in my side-rcu implementation [1, 2]. > It is similar to what SRCU does (per-cpu counter based grace period > tracking), but implemented for userspace. The comments explain why this > works without the memory barrier you identify as useless in SRCU. > > Following my implementation of side-rcu, I reviewed the SRCU comments > and identified that the barrier "/* E */" appears to be useless. I even > discussed this privately with Paul E. McKenney. > > My implementation and comments go further though, and skip the period > "flip" entirely if the first pass observes that all readers (in both > periods) are quiescent. Actually in SRCU, the first pass scans only 1 index, then does the flip, and the second pass scans the second index. Without doing a flip, an index cannot be scanned for forward progress reasons because it is still "active". So I am curious how you can skip flip and still scan both indexes? I will dig more into your implementation to learn more. > The most relevant comment in side-rcu is: > > * The grace period completes when it observes that there are no active > * readers within each of the periods. > * > * The active_readers state is initially true for each period, until the > * grace period observes that no readers are present for each given > * period, at which point the active_readers state becomes false. > > So I agree with the clarifications you propose here, but I think we can > improve the grace period implementation further by clarifying the SRCU > grace period model. Thanks a lot, I am curious how you do the "detection of no new readers" part without globally doing some kind of synchronization. I will dig more into your implementation to learn more. Thanks, - Joel > > Thanks, > > Mathieu > > > [1] https://github.com/efficios/libside/blob/master/src/rcu.h > [2] https://github.com/efficios/libside/blob/master/src/rcu.c > > > > > thanks, > > > > - Joel > > > > Joel Fernandes (Google) (2): > > srcu: Remove comment about prior read lock counts > > srcu: Remove memory barrier "E" as it is not required > > > > kernel/rcu/srcutree.c | 10 ---------- > > 1 file changed, 10 deletions(-) > > > > -- > > 2.39.0.314.g84b9a713c41-goog > > > > -- > Mathieu Desnoyers > EfficiOS Inc. > https://www.efficios.com >
