lkml.org 
[lkml]   [2011]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: INFO: possible circular locking dependency detected
On Fri, Jul 15, 2011 at 12:55:57PM -0400, Steven Rostedt wrote:
> On Fri, 2011-07-15 at 15:07 +0200, Peter Zijlstra wrote:
>
> > OK, so the latter case cannot happen (rcu_preempt_check_callbacks only
> > sets NEED_QS when rcu_read_lock_nesting), we need two interrupts for
> > this to happen.
> >
> > rcu_read_lock()
> >
> > <IRQ>
> > |= RCU_READ_UNLOCK_NEED_QS
> >
> > rcu_read_unlock()
> > __rcu_read_unlock()
> > --rcu_read_lock_nesting;
> > <IRQ>
> > ttwu()
> > rcu_read_lock()
> > rcu_read_unlock()
> > rcu_read_unlock_special()
> > *BANG*
> > rcu_read_unlock_special()
> >
>
> What about this patch? Not even compiled tested.

This runs afoul of the restriction that ->rcu_read_unlock_special must
be updated with irqs disabled, please see below.

I am also missing what the goal is -- I don't immediatly see how this
prevents the scenario that Ed ran into, for example.

Thanx, Paul

> -- Steve
>
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> index 14dc7dd..e3545fa 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.h
> @@ -284,18 +284,17 @@ static struct list_head *rcu_next_node_entry(struct task_struct *t,
> * notify RCU core processing or task having blocked during the RCU
> * read-side critical section.
> */
> -static void rcu_read_unlock_special(struct task_struct *t)
> +static int rcu_read_unlock_special(struct task_struct *t, int special)
> {
> int empty;
> int empty_exp;
> unsigned long flags;
> struct list_head *np;
> struct rcu_node *rnp;
> - int special;
>
> /* NMI handlers cannot block and cannot safely manipulate state. */
> if (in_nmi())
> - return;
> + return special;
>
> local_irq_save(flags);
>
> @@ -303,7 +302,6 @@ static void rcu_read_unlock_special(struct task_struct *t)
> * If RCU core is waiting for this CPU to exit critical section,
> * let it know that we have done so.
> */
> - special = t->rcu_read_unlock_special;
> if (special & RCU_READ_UNLOCK_NEED_QS) {
> rcu_preempt_qs(smp_processor_id());
> }
> @@ -311,7 +309,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
> /* Hardware IRQ handlers cannot block. */
> if (in_irq()) {
> local_irq_restore(flags);
> - return;
> + return special;
> }
>
> /* Clean up if blocked during RCU read-side critical section. */
> @@ -373,6 +371,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
> } else {
> local_irq_restore(flags);
> }
> + return special;
> }
>
> /*
> @@ -385,13 +384,21 @@ static void rcu_read_unlock_special(struct task_struct *t)
> void __rcu_read_unlock(void)
> {
> struct task_struct *t = current;
> + int special;
>
> + special = ACCESS_ONCE(t->rcu_read_unlock_special);
> + /*
> + * Clear special here to prevent interrupts from seeing it
> + * enabled after decrementing lock_nesting and calling
> + * rcu_read_unlock_special().
> + */

Any change to ->rcu_read_unlock_special from an irq handler that happens
here is lost. Changes to ->rcu_read_unlock_special must be done with
irqs disabled. And I hope to avoid irq disabling on the rcu_read_unlock()
fastpath.

> + t->rcu_read_unlock_special = 0;
> barrier(); /* needed if we ever invoke rcu_read_unlock in rcutree.c */
> --t->rcu_read_lock_nesting;
> barrier(); /* decrement before load of ->rcu_read_unlock_special */
> - if (t->rcu_read_lock_nesting == 0 &&
> - unlikely(ACCESS_ONCE(t->rcu_read_unlock_special)))
> - rcu_read_unlock_special(t);
> + if (t->rcu_read_lock_nesting == 0 && special)
> + special = rcu_read_unlock_special(t, special);

And changes to ->rcu_read_unlock_special from an irq handler that happens
here are also lost.

> + t->rcu_read_unlock_special = special;
> #ifdef CONFIG_PROVE_LOCKING
> WARN_ON_ONCE(ACCESS_ONCE(t->rcu_read_lock_nesting) < 0);
> #endif /* #ifdef CONFIG_PROVE_LOCKING */
>
>


\
 
 \ /
  Last update: 2011-07-15 19:05    [W:0.058 / U:1.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site