Re: [PATCH 04/16] rcu: rcutiny: Prevent RCU stall
On Sat, 2013-02-16 at 12:59 -0800, Paul E. McKenney wrote:
>
> > diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
> > index 2b0484a..bac1906 100644
> > --- a/kernel/rcutiny_plugin.h
> > +++ b/kernel/rcutiny_plugin.h
> > @@ -552,7 +552,7 @@ static void rcu_read_unlock_special(struct task_struct
> > *t)
> > rcu_preempt_cpu_qs();
> >
> > /* Hardware IRQ handlers cannot block. */
> > - if (in_irq()) {
> > + if (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_OFFSET)) {
>
> For whatever it is worth, in mainline this is:
>
> if (in_irq() || in_serving_softirq()) {
>
> The definition of in_serving_softirq() is a bit different:
>
> #define in_serving_softirq()(softirq_count() & SOFTIRQ_OFFSET)
>
> This might be due to differences between mainline and -rt, but thought
> it worth calling attention to.
>
Right, and we should avoid open coding HARDIRQ_MASK and SOFTIRQ_OFFSET.
And note, -rt is slowly creeping into mainline. It's better to have
things like this nicely tidied up.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/16] rcu: rcutiny: Prevent RCU stall
On Sat, 2013-02-16 at 12:59 -0800, Paul E. McKenney wrote:
diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
index 2b0484a..bac1906 100644
--- a/kernel/rcutiny_plugin.h
+++ b/kernel/rcutiny_plugin.h
@@ -552,7 +552,7 @@ static void rcu_read_unlock_special(struct task_struct
*t)
rcu_preempt_cpu_qs();
/* Hardware IRQ handlers cannot block. */
- if (in_irq()) {
+ if (preempt_count() (HARDIRQ_MASK | SOFTIRQ_OFFSET)) {
For whatever it is worth, in mainline this is:
if (in_irq() || in_serving_softirq()) {
The definition of in_serving_softirq() is a bit different:
#define in_serving_softirq()(softirq_count() SOFTIRQ_OFFSET)
This might be due to differences between mainline and -rt, but thought
it worth calling attention to.
Right, and we should avoid open coding HARDIRQ_MASK and SOFTIRQ_OFFSET.
And note, -rt is slowly creeping into mainline. It's better to have
things like this nicely tidied up.
-- Steve
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/16] rcu: rcutiny: Prevent RCU stall
On Wed, Feb 13, 2013 at 05:11:59PM +0100, Sebastian Andrzej Siewior wrote:
> From: Thomas Gleixner
>
> rcu_read_unlock_special() checks in_serving_softirq() and leaves early
> when true. On RT this is obviously wrong as softirq processing context
> can be preempted and therefor such a task can be on the gp_tasks
> list. Leaving early here will leave the task on the list and therefor
> block RCU processing forever.
>
> This cannot happen on mainline because softirq processing context
> cannot be preempted and therefor this can never happen at all.
>
> In fact this check looks quite questionable in general. Neither irq
> context nor softirq processing context in mainline can ever be
> preempted in mainline so the special unlock case should not ever be
> invoked in such context. Now the only explanation might be a
> rcu_read_unlock() being interrupted and therefor leave the rcu nest
> count at 0 before the special unlock bit has been cleared. That looks
> fragile. At least it's missing a big fat comment. Paul
>
> See mainline commits: ec433f0c5 and 8762705a for further enlightment.
>
> Reported-by: Kristian Lehmann
> Signed-off-by: Thomas Gleixner
> [bigeasy@linutronix: different in-irq check]
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> kernel/rcutiny_plugin.h |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
> index 2b0484a..bac1906 100644
> --- a/kernel/rcutiny_plugin.h
> +++ b/kernel/rcutiny_plugin.h
> @@ -552,7 +552,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
> rcu_preempt_cpu_qs();
>
> /* Hardware IRQ handlers cannot block. */
> - if (in_irq()) {
> + if (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_OFFSET)) {
For whatever it is worth, in mainline this is:
if (in_irq() || in_serving_softirq()) {
The definition of in_serving_softirq() is a bit different:
#define in_serving_softirq()(softirq_count() & SOFTIRQ_OFFSET)
This might be due to differences between mainline and -rt, but thought
it worth calling attention to.
Thanx, Paul
> local_irq_restore(flags);
> return;
> }
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/16] rcu: rcutiny: Prevent RCU stall
On Wed, Feb 13, 2013 at 05:11:59PM +0100, Sebastian Andrzej Siewior wrote:
From: Thomas Gleixner t...@linutronix.de
rcu_read_unlock_special() checks in_serving_softirq() and leaves early
when true. On RT this is obviously wrong as softirq processing context
can be preempted and therefor such a task can be on the gp_tasks
list. Leaving early here will leave the task on the list and therefor
block RCU processing forever.
This cannot happen on mainline because softirq processing context
cannot be preempted and therefor this can never happen at all.
In fact this check looks quite questionable in general. Neither irq
context nor softirq processing context in mainline can ever be
preempted in mainline so the special unlock case should not ever be
invoked in such context. Now the only explanation might be a
rcu_read_unlock() being interrupted and therefor leave the rcu nest
count at 0 before the special unlock bit has been cleared. That looks
fragile. At least it's missing a big fat comment. Paul
See mainline commits: ec433f0c5 and 8762705a for further enlightment.
Reported-by: Kristian Lehmann krlei...@hs-esslingen.de
Signed-off-by: Thomas Gleixner t...@linutronix.de
[bigeasy@linutronix: different in-irq check]
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
kernel/rcutiny_plugin.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
index 2b0484a..bac1906 100644
--- a/kernel/rcutiny_plugin.h
+++ b/kernel/rcutiny_plugin.h
@@ -552,7 +552,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
rcu_preempt_cpu_qs();
/* Hardware IRQ handlers cannot block. */
- if (in_irq()) {
+ if (preempt_count() (HARDIRQ_MASK | SOFTIRQ_OFFSET)) {
For whatever it is worth, in mainline this is:
if (in_irq() || in_serving_softirq()) {
The definition of in_serving_softirq() is a bit different:
#define in_serving_softirq()(softirq_count() SOFTIRQ_OFFSET)
This might be due to differences between mainline and -rt, but thought
it worth calling attention to.
Thanx, Paul
local_irq_restore(flags);
return;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/16] rcu: rcutiny: Prevent RCU stall
From: Thomas Gleixner
rcu_read_unlock_special() checks in_serving_softirq() and leaves early
when true. On RT this is obviously wrong as softirq processing context
can be preempted and therefor such a task can be on the gp_tasks
list. Leaving early here will leave the task on the list and therefor
block RCU processing forever.
This cannot happen on mainline because softirq processing context
cannot be preempted and therefor this can never happen at all.
In fact this check looks quite questionable in general. Neither irq
context nor softirq processing context in mainline can ever be
preempted in mainline so the special unlock case should not ever be
invoked in such context. Now the only explanation might be a
rcu_read_unlock() being interrupted and therefor leave the rcu nest
count at 0 before the special unlock bit has been cleared. That looks
fragile. At least it's missing a big fat comment. Paul
See mainline commits: ec433f0c5 and 8762705a for further enlightment.
Reported-by: Kristian Lehmann
Signed-off-by: Thomas Gleixner
[bigeasy@linutronix: different in-irq check]
Signed-off-by: Sebastian Andrzej Siewior
---
kernel/rcutiny_plugin.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
index 2b0484a..bac1906 100644
--- a/kernel/rcutiny_plugin.h
+++ b/kernel/rcutiny_plugin.h
@@ -552,7 +552,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
rcu_preempt_cpu_qs();
/* Hardware IRQ handlers cannot block. */
- if (in_irq()) {
+ if (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_OFFSET)) {
local_irq_restore(flags);
return;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/16] rcu: rcutiny: Prevent RCU stall
From: Thomas Gleixner t...@linutronix.de
rcu_read_unlock_special() checks in_serving_softirq() and leaves early
when true. On RT this is obviously wrong as softirq processing context
can be preempted and therefor such a task can be on the gp_tasks
list. Leaving early here will leave the task on the list and therefor
block RCU processing forever.
This cannot happen on mainline because softirq processing context
cannot be preempted and therefor this can never happen at all.
In fact this check looks quite questionable in general. Neither irq
context nor softirq processing context in mainline can ever be
preempted in mainline so the special unlock case should not ever be
invoked in such context. Now the only explanation might be a
rcu_read_unlock() being interrupted and therefor leave the rcu nest
count at 0 before the special unlock bit has been cleared. That looks
fragile. At least it's missing a big fat comment. Paul
See mainline commits: ec433f0c5 and 8762705a for further enlightment.
Reported-by: Kristian Lehmann krlei...@hs-esslingen.de
Signed-off-by: Thomas Gleixner t...@linutronix.de
[bigeasy@linutronix: different in-irq check]
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
kernel/rcutiny_plugin.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
index 2b0484a..bac1906 100644
--- a/kernel/rcutiny_plugin.h
+++ b/kernel/rcutiny_plugin.h
@@ -552,7 +552,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
rcu_preempt_cpu_qs();
/* Hardware IRQ handlers cannot block. */
- if (in_irq()) {
+ if (preempt_count() (HARDIRQ_MASK | SOFTIRQ_OFFSET)) {
local_irq_restore(flags);
return;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
