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