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

Groups > linux.kernel > #1240639 > unrolled thread

[PATCH tip/core/rcu 0/13] Miscellaneous fixes for 4.4

Back to article view | Back to linux.kernel

Contents

  [PATCH tip/core/rcu 0/13] Miscellaneous fixes for 4.4 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
    [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for Tiny and Tree RCU readers "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
      Re: [Kernel networking modules.]  OSI levels 2 & 3, Assistance - If anyone knows anyone in the US. North West region "John D Allen, Leveridge Systems INC"   <john.allen@leveridgesystems.com> - 2015-10-06 18:30 +0200
      Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers Josh Triplett <josh@joshtriplett.org> - 2015-10-06 18:50 +0200
        Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:20 +0200
          Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:50 +0200
          Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 19:50 +0200
            Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers Peter Zijlstra <peterz@infradead.org> - 2015-10-06 22:10 +0200
              Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 22:20 +0200
                Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers Peter Zijlstra <peterz@infradead.org> - 2015-10-06 23:00 +0200
                  Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 23:10 +0200
                    Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers Peter Zijlstra <peterz@infradead.org> - 2015-10-07 09:30 +0200
        Re: [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for  Tiny and Tree RCU readers "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 19:20 +0200
    [PATCH tip/core/rcu 03/13] rcu: Use call_rcu_func_t to replace explicit type equivalents "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
    [PATCH tip/core/rcu 01/13] sched: Export sched_setscheduler_nocheck "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
      [PATCH tip/core/rcu 09/13] rcu: Correct comment for values of ->gp_state field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
      [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless_dereference() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
      [PATCH tip/core/rcu 02/13] rcu: Use rcu_callback_t in call_rcu*() and friends "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
      [PATCH tip/core/rcu 12/13] rcu: Remove deprecated rcu_lockdep_assert() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
      [PATCH tip/core/rcu 06/13] rcu: Add online/offline info to stall warning message "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:20 +0200
        Re: [PATCH tip/core/rcu 06/13] rcu: Add online/offline info to stall  warning message Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:20 +0200
      [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:30 +0200
        Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:20 +0200
          Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 19:40 +0200
            Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:50 +0200
              Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 20:10 +0200
        Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() Peter Zijlstra <peterz@infradead.org> - 2015-10-06 22:10 +0200
          Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 22:20 +0200
            Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() Peter Zijlstra <peterz@infradead.org> - 2015-10-06 22:40 +0200
              Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 23:10 +0200
                Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() Peter Zijlstra <peterz@infradead.org> - 2015-10-07 09:30 +0200
                  Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of  __srcu_read_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 16:20 +0200
      [PATCH tip/core/rcu 05/13] rcu: Eliminate panic when silly boot-time fanout specified "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:30 +0200
      [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:30 +0200
        Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:30 +0200
          Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 19:40 +0200
            Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:40 +0200
        Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() Peter Zijlstra <peterz@infradead.org> - 2015-10-06 22:30 +0200
          Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 23:10 +0200
            Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() Peter Zijlstra <peterz@infradead.org> - 2015-10-07 09:30 +0200
              Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 16:30 +0200
    Re: [PATCH tip/core/rcu 0/13] Miscellaneous fixes for 4.4 Josh Triplett <josh@joshtriplett.org> - 2015-10-06 19:30 +0200
      Re: [PATCH tip/core/rcu 0/13] Miscellaneous fixes for 4.4 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 19:40 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1240760 — Re: [PATCH tip/core/rcu 06/13] rcu: Add online/offline info to stall warning message

On Tue, Oct 06, 2015 at 09:13:41AM -0700, Paul E. McKenney wrote:
> This commit makes the RCU CPU stall warning message print online/offline
> indications immediately after the CPU number.  A "?" indicates global
> offline, a "," global online, and a "!" indicates RCU believes that the
> CPU is offline and "." otherwise, both right after the CPU number.
> So for CPU 10, you would normally see "10,.:" indicating that everything
> believes that the CPU is online.

This explanation doesn't seem to agree with the actual characters used.
These use 'O', 'o', and 'N', instead, with '.' for "true" in all three
cases.  And the output from the code includes a '-' after the CPU
number.

Also, this output needs matching documentation in
src/linux/Documentation/RCU/stallwarn.txt, ideally added as part of the
same commit.

> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcu/tree_plugin.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 06116ae6dfd7..57ed9c13ae5a 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1702,8 +1702,12 @@ static void print_cpu_stall_info(struct rcu_state *rsp, int cpu)
>  		ticks_value = rsp->gpnum - rdp->gpnum;
>  	}
>  	print_cpu_stall_fast_no_hz(fast_no_hz, cpu);
> -	pr_err("\t%d: (%lu %s) idle=%03x/%llx/%d softirq=%u/%u fqs=%ld %s\n",
> -	       cpu, ticks_value, ticks_title,
> +	pr_err("\t%d-%c%c%c: (%lu %s) idle=%03x/%llx/%d softirq=%u/%u fqs=%ld %s\n",
> +	       cpu,
> +	       "O."[!!cpu_online(cpu)],
> +	       "o."[!!(rdp->grpmask & rdp->mynode->qsmaskinit)],
> +	       "N."[!!(rdp->grpmask & rdp->mynode->qsmaskinitnext)],
> +	       ticks_value, ticks_title,
>  	       atomic_read(&rdtp->dynticks) & 0xfff,
>  	       rdtp->dynticks_nesting, rdtp->dynticks_nmi_nesting,
>  	       rdp->softirq_snap, kstat_softirqs_cpu(RCU_SOFTIRQ, cpu),
> -- 
> 2.5.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240664 — [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

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().

Signed-off-by: Paul E. McKenney <paulmck@linux.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240766 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

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.

> Signed-off-by: Paul E. McKenney <paulmck@linux.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240787 — 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@linux.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240810 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Tue, Oct 06, 2015 at 10:36:46AM -0700, Paul E. McKenney wrote:
> 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().

I agree; I just find the idea that someone would need to call
__srcu_read_lock rather than srcu_read_lock odd and worthy of further
understanding. :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240835 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Tue, Oct 06, 2015 at 10:43:11AM -0700, Josh Triplett wrote:
> On Tue, Oct 06, 2015 at 10:36:46AM -0700, Paul E. McKenney wrote:
> > 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().
> 
> I agree; I just find the idea that someone would need to call
> __srcu_read_lock rather than srcu_read_lock odd and worthy of further
> understanding. :)

And they did supply an explanation that seemed satisfactory at the time,
but I cannot find that either.  I clearly need to track that sort of stuff
better!

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1240943 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

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().

Did you not simply want to use: preempt_disable_notrace() ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240959 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Tue, Oct 06, 2015 at 10:07:25PM +0200, Peter Zijlstra 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().
> 
> Did you not simply want to use: preempt_disable_notrace() ?

I believe that tracing the preempt_disable() in srcu_read_lock() and
srcu_read_unlock() is actually a good thing.  Or am I missing your
point?

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1240976 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Tue, Oct 06, 2015 at 01:19:15PM -0700, Paul E. McKenney wrote:
> On Tue, Oct 06, 2015 at 10:07:25PM +0200, Peter Zijlstra 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().
> > 
> > Did you not simply want to use: preempt_disable_notrace() ?
> 
> I believe that tracing the preempt_disable() in srcu_read_lock() and
> srcu_read_unlock() is actually a good thing.  Or am I missing your
> point?

Depends a bit on why we needed this change in the first place -- which,
going by the other branch of this thread, seems lost. However,
preempt_{dis,en}able_notrace() will not end up in any tracer/lockdep and
generate the minimum code that preserves the required semantics.


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

[toc] | [prev] | [next] | [standalone]

#1241003 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Tue, Oct 06, 2015 at 10:32:24PM +0200, Peter Zijlstra wrote:
> On Tue, Oct 06, 2015 at 01:19:15PM -0700, Paul E. McKenney wrote:
> > On Tue, Oct 06, 2015 at 10:07:25PM +0200, Peter Zijlstra 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().
> > > 
> > > Did you not simply want to use: preempt_disable_notrace() ?
> > 
> > I believe that tracing the preempt_disable() in srcu_read_lock() and
> > srcu_read_unlock() is actually a good thing.  Or am I missing your
> > point?
> 
> Depends a bit on why we needed this change in the first place -- which,
> going by the other branch of this thread, seems lost. However,
> preempt_{dis,en}able_notrace() will not end up in any tracer/lockdep and
> generate the minimum code that preserves the required semantics.

True enough!  But can all architectures locate the TIF in all contexts?

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1241196 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Tue, Oct 06, 2015 at 02:03:48PM -0700, Paul E. McKenney wrote:
> On Tue, Oct 06, 2015 at 10:32:24PM +0200, Peter Zijlstra wrote:
> > On Tue, Oct 06, 2015 at 01:19:15PM -0700, Paul E. McKenney wrote:
> > > On Tue, Oct 06, 2015 at 10:07:25PM +0200, Peter Zijlstra 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().
> > > > 
> > > > Did you not simply want to use: preempt_disable_notrace() ?
> > > 
> > > I believe that tracing the preempt_disable() in srcu_read_lock() and
> > > srcu_read_unlock() is actually a good thing.  Or am I missing your
> > > point?
> > 
> > Depends a bit on why we needed this change in the first place -- which,
> > going by the other branch of this thread, seems lost. However,
> > preempt_{dis,en}able_notrace() will not end up in any tracer/lockdep and
> > generate the minimum code that preserves the required semantics.
> 
> True enough!  But can all architectures locate the TIF in all contexts?

They had better, otherwise we have a problem with NMIs touching it :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241544 — Re: [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock()

On Wed, Oct 07, 2015 at 09:20:38AM +0200, Peter Zijlstra wrote:
> On Tue, Oct 06, 2015 at 02:03:48PM -0700, Paul E. McKenney wrote:
> > On Tue, Oct 06, 2015 at 10:32:24PM +0200, Peter Zijlstra wrote:
> > > On Tue, Oct 06, 2015 at 01:19:15PM -0700, Paul E. McKenney wrote:
> > > > On Tue, Oct 06, 2015 at 10:07:25PM +0200, Peter Zijlstra 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().
> > > > > 
> > > > > Did you not simply want to use: preempt_disable_notrace() ?
> > > > 
> > > > I believe that tracing the preempt_disable() in srcu_read_lock() and
> > > > srcu_read_unlock() is actually a good thing.  Or am I missing your
> > > > point?
> > > 
> > > Depends a bit on why we needed this change in the first place -- which,
> > > going by the other branch of this thread, seems lost. However,
> > > preempt_{dis,en}able_notrace() will not end up in any tracer/lockdep and
> > > generate the minimum code that preserves the required semantics.
> > 
> > True enough!  But can all architectures locate the TIF in all contexts?
> 
> They had better, otherwise we have a problem with NMIs touching it :-)

Fair enough!  ;-)

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1240669 — [PATCH tip/core/rcu 05/13] rcu: Eliminate panic when silly boot-time fanout specified

This commit loosens rcutree.rcu_fanout_leaf range checks
and replaces a panic() with a fallback to compile-time values.
This fallback is accompanied by a WARN_ON(), and both occur when the
rcutree.rcu_fanout_leaf value is too small to accommodate the number of
CPUs.  For example, given the current four-level limit for the rcu_node
tree, a system with more than 16 CPUs built with CONFIG_FANOUT=2 must
have rcutree.rcu_fanout_leaf larger than 2.

Reported-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/kernel-parameters.txt |  9 ++++++---
 kernel/rcu/tree.c                   | 20 +++++++++++---------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 22a4b687ea5b..23ec96877311 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3074,9 +3074,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			cache-to-cache transfer latencies.
 
 	rcutree.rcu_fanout_leaf= [KNL]
-			Increase the number of CPUs assigned to each
-			leaf rcu_node structure.  Useful for very large
-			systems.
+			Change the number of CPUs assigned to each
+			leaf rcu_node structure.  Useful for very
+			large systems, which will choose the value 64,
+			and for NUMA systems with large remote-access
+			latencies, which will choose a value aligned
+			with the appropriate hardware boundaries.
 
 	rcutree.jiffies_till_sched_qs= [KNL]
 			Set required age in jiffies for a
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 93c0f23c3e45..713eb92314b4 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -4230,13 +4230,12 @@ static void __init rcu_init_geometry(void)
 		rcu_fanout_leaf, nr_cpu_ids);
 
 	/*
-	 * The boot-time rcu_fanout_leaf parameter is only permitted
-	 * to increase the leaf-level fanout, not decrease it.  Of course,
-	 * the leaf-level fanout cannot exceed the number of bits in
-	 * the rcu_node masks.  Complain and fall back to the compile-
-	 * time values if these limits are exceeded.
+	 * The boot-time rcu_fanout_leaf parameter must be at least two
+	 * and cannot exceed the number of bits in the rcu_node masks.
+	 * Complain and fall back to the compile-time values if this
+	 * limit is exceeded.
 	 */
-	if (rcu_fanout_leaf < RCU_FANOUT_LEAF ||
+	if (rcu_fanout_leaf < 2 ||
 	    rcu_fanout_leaf > sizeof(unsigned long) * 8) {
 		rcu_fanout_leaf = RCU_FANOUT_LEAF;
 		WARN_ON(1);
@@ -4253,10 +4252,13 @@ static void __init rcu_init_geometry(void)
 
 	/*
 	 * The tree must be able to accommodate the configured number of CPUs.
-	 * If this limit is exceeded than we have a serious problem elsewhere.
+	 * If this limit is exceeded, fall back to the compile-time values.
 	 */
-	if (nr_cpu_ids > rcu_capacity[RCU_NUM_LVLS - 1])
-		panic("rcu_init_geometry: rcu_capacity[] is too small");
+	if (nr_cpu_ids > rcu_capacity[RCU_NUM_LVLS - 1]) {
+		rcu_fanout_leaf = RCU_FANOUT_LEAF;
+		WARN_ON(1);
+		return;
+	}
 
 	/* Calculate the number of levels in the tree. */
 	for (i = 0; nr_cpu_ids > rcu_capacity[i]; i++) {
-- 
2.5.2

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

[toc] | [prev] | [next] | [standalone]

#1240671 — [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

This commit adds an rcu_pointer_handoff() that is intended to mark
situations where a structure's protection transitions from RCU to some
other mechanism (locking, reference counting, whatever).  These markings
should allow external tools to more easily spot bugs involving leaking
pointers out of RCU read-side critical sections.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 6c3ceceb6148..587eb057e2fa 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -813,6 +813,28 @@ static inline void rcu_preempt_sleep_check(void)
 #define rcu_dereference_sched(p) rcu_dereference_sched_check(p, 0)
 
 /**
+ * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
+ * @p: The pointer to hand off
+ *
+ * This is simply an identity function, but it documents where a pointer
+ * is handed off from RCU to some other synchronization mechanism, for
+ * example, reference counting or locking.  In C11, it would map to
+ * kill_dependency().  It could be used as follows:
+ *
+ *	rcu_read_lock();
+ *	p = rcu_dereference(gp);
+ *	long_lived = is_long_lived(p);
+ *	if (long_lived) {
+ *		if (!atomic_inc_not_zero(p->refcnt))
+ *			long_lived = false;
+ *		else
+ *			p = rcu_pointer_handoff(p);
+ *	}
+ *	rcu_read_unlock();
+ */
+#define rcu_pointer_handoff(p) (p)
+
+/**
  * rcu_read_lock() - mark the beginning of an RCU read-side critical section
  *
  * When synchronize_rcu() is invoked on one CPU while other CPUs
-- 
2.5.2

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

[toc] | [prev] | [next] | [standalone]

#1240776 — Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

On Tue, Oct 06, 2015 at 09:13:45AM -0700, Paul E. McKenney wrote:
> This commit adds an rcu_pointer_handoff() that is intended to mark
> situations where a structure's protection transitions from RCU to some
> other mechanism (locking, reference counting, whatever).  These markings
> should allow external tools to more easily spot bugs involving leaking
> pointers out of RCU read-side critical sections.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Shouldn't this expect the __rcu address space on the pointer, and cast
away the __rcu with __force?

>  include/linux/rcupdate.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 6c3ceceb6148..587eb057e2fa 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -813,6 +813,28 @@ static inline void rcu_preempt_sleep_check(void)
>  #define rcu_dereference_sched(p) rcu_dereference_sched_check(p, 0)
>  
>  /**
> + * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
> + * @p: The pointer to hand off
> + *
> + * This is simply an identity function, but it documents where a pointer
> + * is handed off from RCU to some other synchronization mechanism, for
> + * example, reference counting or locking.  In C11, it would map to
> + * kill_dependency().  It could be used as follows:
> + *
> + *	rcu_read_lock();
> + *	p = rcu_dereference(gp);
> + *	long_lived = is_long_lived(p);
> + *	if (long_lived) {
> + *		if (!atomic_inc_not_zero(p->refcnt))
> + *			long_lived = false;
> + *		else
> + *			p = rcu_pointer_handoff(p);
> + *	}
> + *	rcu_read_unlock();
> + */
> +#define rcu_pointer_handoff(p) (p)
> +
> +/**
>   * rcu_read_lock() - mark the beginning of an RCU read-side critical section
>   *
>   * When synchronize_rcu() is invoked on one CPU while other CPUs
> -- 
> 2.5.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240784 — Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

On Tue, Oct 06, 2015 at 10:21:28AM -0700, Josh Triplett wrote:
> On Tue, Oct 06, 2015 at 09:13:45AM -0700, Paul E. McKenney wrote:
> > This commit adds an rcu_pointer_handoff() that is intended to mark
> > situations where a structure's protection transitions from RCU to some
> > other mechanism (locking, reference counting, whatever).  These markings
> > should allow external tools to more easily spot bugs involving leaking
> > pointers out of RCU read-side critical sections.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Shouldn't this expect the __rcu address space on the pointer, and cast
> away the __rcu with __force?

I do not believe so, given that the __rcu was already removed by a preceding
rcu_dereference().  Or am I missing something?

							Thanx, Paul

> >  include/linux/rcupdate.h | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 6c3ceceb6148..587eb057e2fa 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -813,6 +813,28 @@ static inline void rcu_preempt_sleep_check(void)
> >  #define rcu_dereference_sched(p) rcu_dereference_sched_check(p, 0)
> >  
> >  /**
> > + * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
> > + * @p: The pointer to hand off
> > + *
> > + * This is simply an identity function, but it documents where a pointer
> > + * is handed off from RCU to some other synchronization mechanism, for
> > + * example, reference counting or locking.  In C11, it would map to
> > + * kill_dependency().  It could be used as follows:
> > + *
> > + *	rcu_read_lock();
> > + *	p = rcu_dereference(gp);
> > + *	long_lived = is_long_lived(p);
> > + *	if (long_lived) {
> > + *		if (!atomic_inc_not_zero(p->refcnt))
> > + *			long_lived = false;
> > + *		else
> > + *			p = rcu_pointer_handoff(p);
> > + *	}
> > + *	rcu_read_unlock();
> > + */
> > +#define rcu_pointer_handoff(p) (p)
> > +
> > +/**
> >   * rcu_read_lock() - mark the beginning of an RCU read-side critical section
> >   *
> >   * When synchronize_rcu() is invoked on one CPU while other CPUs
> > -- 
> > 2.5.2
> > 
> 

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

[toc] | [prev] | [next] | [standalone]

#1240798 — Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

On Tue, Oct 06, 2015 at 10:31:52AM -0700, Paul E. McKenney wrote:
> On Tue, Oct 06, 2015 at 10:21:28AM -0700, Josh Triplett wrote:
> > On Tue, Oct 06, 2015 at 09:13:45AM -0700, Paul E. McKenney wrote:
> > > This commit adds an rcu_pointer_handoff() that is intended to mark
> > > situations where a structure's protection transitions from RCU to some
> > > other mechanism (locking, reference counting, whatever).  These markings
> > > should allow external tools to more easily spot bugs involving leaking
> > > pointers out of RCU read-side critical sections.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Shouldn't this expect the __rcu address space on the pointer, and cast
> > away the __rcu with __force?
> 
> I do not believe so, given that the __rcu was already removed by a preceding
> rcu_dereference().  Or am I missing something?

Ah, I see.  Per the example, you don't call this on an __rcu pointer
directly, only on a pointer you've already obtained from RCU after
giving it a lifetime of its own.  Nevermind.

> > >  include/linux/rcupdate.h | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > index 6c3ceceb6148..587eb057e2fa 100644
> > > --- a/include/linux/rcupdate.h
> > > +++ b/include/linux/rcupdate.h
> > > @@ -813,6 +813,28 @@ static inline void rcu_preempt_sleep_check(void)
> > >  #define rcu_dereference_sched(p) rcu_dereference_sched_check(p, 0)
> > >  
> > >  /**
> > > + * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
> > > + * @p: The pointer to hand off
> > > + *
> > > + * This is simply an identity function, but it documents where a pointer
> > > + * is handed off from RCU to some other synchronization mechanism, for
> > > + * example, reference counting or locking.  In C11, it would map to
> > > + * kill_dependency().  It could be used as follows:
> > > + *
> > > + *	rcu_read_lock();
> > > + *	p = rcu_dereference(gp);
> > > + *	long_lived = is_long_lived(p);
> > > + *	if (long_lived) {
> > > + *		if (!atomic_inc_not_zero(p->refcnt))
> > > + *			long_lived = false;
> > > + *		else
> > > + *			p = rcu_pointer_handoff(p);
> > > + *	}
> > > + *	rcu_read_unlock();
> > > + */
> > > +#define rcu_pointer_handoff(p) (p)
> > > +
> > > +/**
> > >   * rcu_read_lock() - mark the beginning of an RCU read-side critical section
> > >   *
> > >   * When synchronize_rcu() is invoked on one CPU while other CPUs
> > > -- 
> > > 2.5.2
> > > 
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240968 — Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

On Tue, Oct 06, 2015 at 09:13:45AM -0700, Paul E. McKenney wrote:
>  /**
> + * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
> + * @p: The pointer to hand off
> + *
> + * This is simply an identity function, but it documents where a pointer
> + * is handed off from RCU to some other synchronization mechanism, for
> + * example, reference counting or locking.  In C11, it would map to
> + * kill_dependency().  It could be used as follows:
> + *
> + *	rcu_read_lock();
> + *	p = rcu_dereference(gp);
> + *	long_lived = is_long_lived(p);
> + *	if (long_lived) {
> + *		if (!atomic_inc_not_zero(p->refcnt))
> + *			long_lived = false;
> + *		else
> + *			p = rcu_pointer_handoff(p);
> + *	}
> + *	rcu_read_unlock();
> + */
> +#define rcu_pointer_handoff(p) (p)

Will you actually be using this? It seems a tad pointless to add if you
don't.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240997 — Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

On Tue, Oct 06, 2015 at 10:27:41PM +0200, Peter Zijlstra wrote:
> On Tue, Oct 06, 2015 at 09:13:45AM -0700, Paul E. McKenney wrote:
> >  /**
> > + * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
> > + * @p: The pointer to hand off
> > + *
> > + * This is simply an identity function, but it documents where a pointer
> > + * is handed off from RCU to some other synchronization mechanism, for
> > + * example, reference counting or locking.  In C11, it would map to
> > + * kill_dependency().  It could be used as follows:
> > + *
> > + *	rcu_read_lock();
> > + *	p = rcu_dereference(gp);
> > + *	long_lived = is_long_lived(p);
> > + *	if (long_lived) {
> > + *		if (!atomic_inc_not_zero(p->refcnt))
> > + *			long_lived = false;
> > + *		else
> > + *			p = rcu_pointer_handoff(p);
> > + *	}
> > + *	rcu_read_unlock();
> > + */
> > +#define rcu_pointer_handoff(p) (p)
> 
> Will you actually be using this? It seems a tad pointless to add if you
> don't.

Some of the LLVM guys believe that they can diagnose RCU pointer leaks
if this is used.  But yes, it does need to be used.

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1241191 — Re: [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff()

On Tue, Oct 06, 2015 at 02:02:43PM -0700, Paul E. McKenney wrote:
> On Tue, Oct 06, 2015 at 10:27:41PM +0200, Peter Zijlstra wrote:
> > On Tue, Oct 06, 2015 at 09:13:45AM -0700, Paul E. McKenney wrote:
> > >  /**
> > > + * rcu_pointer_handoff() - Hand off a pointer from RCU to other mechanism
> > > + * @p: The pointer to hand off
> > > + *
> > > + * This is simply an identity function, but it documents where a pointer
> > > + * is handed off from RCU to some other synchronization mechanism, for
> > > + * example, reference counting or locking.  In C11, it would map to
> > > + * kill_dependency().  It could be used as follows:
> > > + *
> > > + *	rcu_read_lock();
> > > + *	p = rcu_dereference(gp);
> > > + *	long_lived = is_long_lived(p);
> > > + *	if (long_lived) {
> > > + *		if (!atomic_inc_not_zero(p->refcnt))
> > > + *			long_lived = false;
> > > + *		else
> > > + *			p = rcu_pointer_handoff(p);
> > > + *	}
> > > + *	rcu_read_unlock();
> > > + */
> > > +#define rcu_pointer_handoff(p) (p)
> > 
> > Will you actually be using this? It seems a tad pointless to add if you
> > don't.
> 
> Some of the LLVM guys believe that they can diagnose RCU pointer leaks
> if this is used.  But yes, it does need to be used.

The thing is, I'm not convinced this is a 'sane' interface. Its _far_
too easy to forget. It doesn't make any kind of sense either, which is
part of why its hard to remember.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web