linux-kernel - Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151006173646.GJ3910@linux.vnet.ibm.com>
Date:	Tue, 6 Oct 2015 10:36:46 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Josh Triplett <josh@...htriplett.org>
Cc:	linux-kernel@...r.kernel.org, mingo@...nel.org,
	jiangshanlai@...il.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
	tglx@...utronix.de, peterz@...radead.org, rostedt@...dmis.org,
	dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
	fweisbec@...il.com, oleg@...hat.com, bobby.prani@...il.com
Subject: Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of
 __srcu_read_lock()

On Tue, Oct 06, 2015 at 10:18:39AM -0700, Josh Triplett wrote:
> On Tue, Oct 06, 2015 at 09:13:42AM -0700, Paul E. McKenney wrote:
> > Currently, __srcu_read_lock() cannot be invoked from restricted
> > environments because it contains calls to preempt_disable() and
> > preempt_enable(), both of which can invoke lockdep, which is a bad
> > idea in some restricted execution modes.  This commit therefore moves
> > the preempt_disable() and preempt_enable() from __srcu_read_lock()
> > to srcu_read_lock().  It also inserts the preempt_disable() and
> > preempt_enable() around the call to __srcu_read_lock() in do_exit().
> 
> What restricted environments do you intend to invoke
> __srcu_read_lock from?
> 
> This change seems fine, but I don't see any change in this patch series
> that needs this, hence my curiosity.

Someone asked me for it, and now I cannot find it.  :-(

Something to the effect of when running unmapped during exception entry
or something like that.  I guess one way to find out would be to remove
the commit and see who complained, but on the other hand, it arguably
makes more sense to have only the bare mechanism is __srcu_read_lock()
and put the environmental protection into srcu_read_lock().

							Thanx, Paul

> > Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > ---
> >  include/linux/srcu.h | 5 ++++-
> >  kernel/exit.c        | 2 ++
> >  kernel/rcu/srcu.c    | 2 --
> >  3 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index bdeb4567b71e..f5f80c5643ac 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -215,8 +215,11 @@ static inline int srcu_read_lock_held(struct srcu_struct *sp)
> >   */
> >  static inline int srcu_read_lock(struct srcu_struct *sp) __acquires(sp)
> >  {
> > -	int retval = __srcu_read_lock(sp);
> > +	int retval;
> >  
> > +	preempt_disable();
> > +	retval = __srcu_read_lock(sp);
> > +	preempt_enable();
> >  	rcu_lock_acquire(&(sp)->dep_map);
> >  	return retval;
> >  }
> > diff --git a/kernel/exit.c b/kernel/exit.c
> > index ea95ee1b5ef7..0e93b63bbc59 100644
> > --- a/kernel/exit.c
> > +++ b/kernel/exit.c
> > @@ -761,7 +761,9 @@ void do_exit(long code)
> >  	 */
> >  	flush_ptrace_hw_breakpoint(tsk);
> >  
> > +	TASKS_RCU(preempt_disable());
> >  	TASKS_RCU(tasks_rcu_i = __srcu_read_lock(&tasks_rcu_exit_srcu));
> > +	TASKS_RCU(preempt_enable());
> >  	exit_notify(tsk, group_dead);
> >  	proc_exit_connector(tsk);
> >  #ifdef CONFIG_NUMA
> > diff --git a/kernel/rcu/srcu.c b/kernel/rcu/srcu.c
> > index 9e6122540d28..a63a1ea5a41b 100644
> > --- a/kernel/rcu/srcu.c
> > +++ b/kernel/rcu/srcu.c
> > @@ -298,11 +298,9 @@ int __srcu_read_lock(struct srcu_struct *sp)
> >  	int idx;
> >  
> >  	idx = READ_ONCE(sp->completed) & 0x1;
> > -	preempt_disable();
> >  	__this_cpu_inc(sp->per_cpu_ref->c[idx]);
> >  	smp_mb(); /* B */  /* Avoid leaking the critical section. */
> >  	__this_cpu_inc(sp->per_cpu_ref->seq[idx]);
> > -	preempt_enable();
> >  	return idx;
> >  }
> >  EXPORT_SYMBOL_GPL(__srcu_read_lock);
> > -- 
> > 2.5.2
> > 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/