Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1658748

Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU from both process and interrupt context

From "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Newsgroups linux.kernel
Subject Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU from both process and interrupt context
Date 2017-06-06 15:00 +0200
Message-ID <tPmfv-vX-1@gated-at.bofh.it> (permalink)
References <tP8me-aN-23@gated-at.bofh.it> <tP8vT-e0-7@gated-at.bofh.it> <tPkx4-7Uz-35@gated-at.bofh.it> <tPlt8-aF-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Jun 06, 2017 at 02:01:54PM +0200, Paolo Bonzini wrote:
> 
> 
> On 06/06/2017 13:09, Peter Zijlstra wrote:
> >> diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> >> index 36e1f82faed1..681bf6bc04a5 100644
> >> --- a/kernel/rcu/srcutiny.c
> >> +++ b/kernel/rcu/srcutiny.c
> >> @@ -35,8 +35,8 @@
> >>  
> >>  static int init_srcu_struct_fields(struct srcu_struct *sp)
> >>  {
> >> -	sp->srcu_lock_nesting[0] = 0;
> >> -	sp->srcu_lock_nesting[1] = 0;
> >> +	atomic_set(&sp->srcu_lock_nesting[0], 0);
> >> +	atomic_set(&sp->srcu_lock_nesting[1], 0);
> >>  	init_swait_queue_head(&sp->srcu_wq);
> >>  	sp->srcu_gp_seq = 0;
> >>  	rcu_segcblist_init(&sp->srcu_cblist);
> >> @@ -86,7 +86,8 @@ EXPORT_SYMBOL_GPL(init_srcu_struct);
> >>   */
> >>  void cleanup_srcu_struct(struct srcu_struct *sp)
> >>  {
> >> -	WARN_ON(sp->srcu_lock_nesting[0] || sp->srcu_lock_nesting[1]);
> >> +	WARN_ON(atomic_read(&sp->srcu_lock_nesting[0]) ||
> >> +		atomic_read(&sp->srcu_lock_nesting[1]));
> >>  	flush_work(&sp->srcu_work);
> >>  	WARN_ON(rcu_seq_state(sp->srcu_gp_seq));
> >>  	WARN_ON(sp->srcu_gp_running);
> >> @@ -97,7 +98,7 @@ EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
> >>  
> >>  /*
> >>   * Counts the new reader in the appropriate per-CPU element of the
> >> - * srcu_struct.  Must be called from process context.
> >> + * srcu_struct.
> >>   * Returns an index that must be passed to the matching srcu_read_unlock().
> >>   */
> >>  int __srcu_read_lock(struct srcu_struct *sp)
> >> @@ -105,21 +106,19 @@ int __srcu_read_lock(struct srcu_struct *sp)
> >>  	int idx;
> >>  
> >>  	idx = READ_ONCE(sp->srcu_idx);
> >> -	WRITE_ONCE(sp->srcu_lock_nesting[idx], sp->srcu_lock_nesting[idx] + 1);
> >> +	atomic_inc(&sp->srcu_lock_nesting[idx]);
> >>  	return idx;
> >>  }
> >>  EXPORT_SYMBOL_GPL(__srcu_read_lock);
> >>  
> >>  /*
> >>   * Removes the count for the old reader from the appropriate element of
> >> - * the srcu_struct.  Must be called from process context.
> >> + * the srcu_struct.
> >>   */
> >>  void __srcu_read_unlock(struct srcu_struct *sp, int idx)
> >>  {
> >> -	int newval = sp->srcu_lock_nesting[idx] - 1;
> >> -
> >> -	WRITE_ONCE(sp->srcu_lock_nesting[idx], newval);
> >> -	if (!newval && READ_ONCE(sp->srcu_gp_waiting))
> >> +	if (atomic_dec_return_relaxed(&sp->srcu_lock_nesting[idx]) == 0 &&
> >> +	    READ_ONCE(sp->srcu_gp_waiting))
> >>  		swake_up(&sp->srcu_wq);
> >>  }
> >>  EXPORT_SYMBOL_GPL(__srcu_read_unlock);
> >> @@ -148,7 +147,7 @@ void srcu_drive_gp(struct work_struct *wp)
> >>  	idx = sp->srcu_idx;
> >>  	WRITE_ONCE(sp->srcu_idx, !sp->srcu_idx);
> >>  	WRITE_ONCE(sp->srcu_gp_waiting, true);  /* srcu_read_unlock() wakes! */
> >> -	swait_event(sp->srcu_wq, !READ_ONCE(sp->srcu_lock_nesting[idx]));
> >> +	swait_event(sp->srcu_wq, !atomic_read(&sp->srcu_lock_nesting[idx]));
> >>  	WRITE_ONCE(sp->srcu_gp_waiting, false); /* srcu_read_unlock() cheap. */
> >>  	rcu_seq_end(&sp->srcu_gp_seq);
> > 
> > I'm not entirely sure this is actually needed. TINY_SRCU is !PREEMPT &&
> > !SMP. So that means all we need is to be safe from IRQs.
> > 
> > Now, do we (want) support things like:
> > 
> > <IRQ>
> >   srcu_read_lock();
> > </IRQ>
> > 
> >   srcu_read_lock();
> > 
> >   srcu_read_unlock();
> > 
> > <IRQ>
> >   srcu_read_unlock();
> > </IRC>
> > 
> > 
> > _OR_
> > 
> > do we already (or want to) mandate that SRCU usage in IRQs must be
> > balanced? That is, if it is used from IRQ context it must do an equal
> > amount of srcu_read_lock() and srcu_read_unlock()s?
> > 
> > Because if we have the balance requirement (as we do for
> > preempt_disable()) then even on load-store architectures the current
> > code should be sufficient (since if an interrupt does as many dec's as
> > it does inc's, the actual value will not change over an interrupt, and
> > our load from before the interrupt is still valid).
> 
> Good point!  So the srcutiny part should not be necessary.  I'll reply
> to the other email now.

Good analysis, Peter!

So the only part of this patch that is needed is the changes to the
comments, right?  ;-)

							Thanx, Paul

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH RFC tip/core/rcu 0/2] srcu: All SRCU readers from both  process and irq "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 00:10 +0200
  [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 00:20 +0200
    Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 13:00 +0200
      Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 15:00 +0200
      Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Paolo Bonzini <pbonzini@redhat.com> - 2017-06-06 15:10 +0200
        Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Christian Borntraeger <borntraeger@de.ibm.com> - 2017-06-06 16:50 +0200
          Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-06-06 17:30 +0200
            Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Christian Borntraeger <borntraeger@de.ibm.com> - 2017-06-06 17:40 +0200
              Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 18:00 +0200
            Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 18:20 +0200
              Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 19:10 +0200
              Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-06-06 19:30 +0200
          Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 18:20 +0200
        Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 18:10 +0200
    Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 13:10 +0200
      Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Paolo Bonzini <pbonzini@redhat.com> - 2017-06-06 14:10 +0200
        Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 15:00 +0200
          Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 18:00 +0200
            Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 18:00 +0200
    Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 19:30 +0200
      Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 20:00 +0200
        Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context Peter Zijlstra <peterz@infradead.org> - 2017-06-06 20:10 +0200
          Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU  from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 20:30 +0200
  [PATCH RFC tip/core/rcu 2/2] srcu: Allow use of Classic SRCU from both process and interrupt context "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 00:20 +0200
  Re: [PATCH RFC tip/core/rcu 0/2] srcu: All SRCU readers from both  process and irq "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-06 19:10 +0200

csiph-web