Messages in this thread | | | Date | Wed, 15 Jul 2015 20:41:03 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/7] rcu: Create rcu_sync infrastructure |
| |
On 07/15, Paul E. McKenney wrote: > > On Sun, Jul 12, 2015 at 01:35:48AM +0200, Oleg Nesterov wrote: > > It is functionally equivalent to > > > > struct rcu_sync_struct { > > atomic_t counter; > > }; > > > > static inline bool rcu_sync_is_idle(struct rcu_sync_struct *rss) > > { > > If you add an smp_mb() here...
I don't think so, please see below...
> > static inline void rcu_sync_exit(struct rcu_sync_struct *rss) > > { > > synchronize_sched(); > > You should be able to demote the above synchronize_sched() to an > smp_mb__before_atomic(). Even rare writes should make this tradeoff > worthwhile.
This is irrelevant I think, this (pseudo) code just tries to explain what this interface does.
> > +static inline bool rcu_sync_is_idle(struct rcu_sync_struct *rss) > > +{ > > smp_mb(); /* A: Ensure that reader sees last update. */ > /* Pairs with B. */ >
Let me remind you about your f0a0e6f282c72247e7c8ec "rcu: Clarify memory-ordering properties of grace-period primitives" documentation patch ;)
We do not need any barrier, assuming that this is called under preempt_disable/etc.
rcu_sync_is_idle() becomes true after another gp pass. The reader should see all updates after that.
> > +void rcu_sync_exit(struct rcu_sync_struct *rss) > > +{ > > + spin_lock_irq(&rss->rss_lock); > > smp_mb(); /* B: Make sure next readers see critical section. */ > /* Pairs with A. */ > > > + if (!--rss->gp_count) { > > At which point, I believe you can ditch the callback entirely, along > with ->cb_state. > > So, what am I missing here?
Please see above. We start anothe gp before "unlock" to avoid mb's in the reader's code.
> Are readers really so frequent that the > added read-side memory barrier is visible?
But this code is heavily optimized for the readers. And please see another discussion about sb_writers and percpu_rw_semaphore. I was suprized, but mb() in sb_start_write() is actually noticeable.
> Given that the current > code forces the readers to grab ->rss_lock
Where? the readers never take this lock.
Oleg.
| |