Re: [PATCH RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Steven Rostedt
On Fri, 24 Jan 2014 16:19:36 -0800
"Paul E. McKenney"  wrote:
 
> Failing to invoke rsp_wakeup() when it was needed could potentially
> stop RCU grace periods from happening, so having rsp_wakeup() happen
> when it is needed is pretty important...
> 
> But I would guess that you knew that already.  ;-)

Yep, I did. But it's always nice to hear confirmation, which is why I
Cc'd you ;-)

-- 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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Paul E. McKenney
On Fri, Jan 24, 2014 at 03:35:42PM -0500, Steven Rostedt wrote:
> On Fri, 24 Jan 2014 21:20:39 +0100
> Sebastian Andrzej Siewior  wrote:
> 
> > * Steven Rostedt | 2014-01-24 15:09:33 [-0500]:
> > 
> > >[ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
> > >  from the interrupt in -rt is a bad thing. Here we simply raise the
> > >  softirq if there's irq work to do. This too boots on my i7 ]
> > 
> > It is okay in general because most of the users should not run in bare
> > interrupt context. The only exception here is the nohz_full_kick_work
> > thing.
> 
> I know we discussed this on IRC, but I wanted to publicly state that
> the missing irq work callback was the RCU's rsp_wakeup() function.

Failing to invoke rsp_wakeup() when it was needed could potentially
stop RCU grace periods from happening, so having rsp_wakeup() happen
when it is needed is pretty important...

But I would guess that you knew that already.  ;-)

Thanx, Paul

--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Sebastian Andrzej Siewior
On 01/24/2014 09:35 PM, Steven Rostedt wrote:
> 
> I know we discussed this on IRC, but I wanted to publicly state that
> the missing irq work callback was the RCU's rsp_wakeup() function.

Let me add that part to that commit message since I can't find it.

> 
> -- Steve

Sebastian

--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Steven Rostedt
On Fri, 24 Jan 2014 21:20:39 +0100
Sebastian Andrzej Siewior  wrote:

> * Steven Rostedt | 2014-01-24 15:09:33 [-0500]:
> 
> >[ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
> >  from the interrupt in -rt is a bad thing. Here we simply raise the
> >  softirq if there's irq work to do. This too boots on my i7 ]
> 
> It is okay in general because most of the users should not run in bare
> interrupt context. The only exception here is the nohz_full_kick_work
> thing.
> 

I know we discussed this on IRC, but I wanted to publicly state that
the missing irq work callback was the RCU's rsp_wakeup() function.

-- 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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Sebastian Andrzej Siewior
* Steven Rostedt | 2014-01-24 15:09:33 [-0500]:

>[ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
>  from the interrupt in -rt is a bad thing. Here we simply raise the
>  softirq if there's irq work to do. This too boots on my i7 ]

It is okay in general because most of the users should not run in bare
interrupt context. The only exception here is the nohz_full_kick_work
thing.

>After trying hard to figure out why my i7 box was locking up with the
>new active_timers code, that does not run the timer softirq if there
>are no active timers, I took an extra look at the softirq handler and
>noticed that it doesn't just run timer softirqs, it also runs irq work.
>
>This was the bug that was locking up the system. It wasn't missing a
>timer, it was missing irq work. By always doing the irq work callbacks,
>the system boots fine.
>
>No need to check for defined(CONFIG_IRQ_WORK). When that's not set the
>"irq_work_needs_cpu()" is a static inline that returns false.
>
>Signed-off-by: Steven Rostedt 

Thank you Steven, this makes sense.

Sebastian
--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Steven Rostedt
[ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
  from the interrupt in -rt is a bad thing. Here we simply raise the
  softirq if there's irq work to do. This too boots on my i7 ]

After trying hard to figure out why my i7 box was locking up with the
new active_timers code, that does not run the timer softirq if there
are no active timers, I took an extra look at the softirq handler and
noticed that it doesn't just run timer softirqs, it also runs irq work.

This was the bug that was locking up the system. It wasn't missing a
timer, it was missing irq work. By always doing the irq work callbacks,
the system boots fine.

No need to check for defined(CONFIG_IRQ_WORK). When that's not set the
"irq_work_needs_cpu()" is a static inline that returns false.

Signed-off-by: Steven Rostedt 

diff --git a/kernel/timer.c b/kernel/timer.c
index 46467be..c01a0d2 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1464,8 +1464,13 @@ void run_local_timers(void)
raise_softirq(TIMER_SOFTIRQ);
return;
}
-   if (!base->active_timers)
-   goto out;
+   if (!base->active_timers) {
+#ifdef CONFIG_PREEMPT_RT_FULL
+   /* On RT, irq work runs from softirq */
+   if (!irq_work_needs_cpu())
+#endif
+   goto out;
+   }
 
/* Check whether the next pending timer has expired */
if (time_before_eq(base->next_timer, jiffies))
--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Steven Rostedt
[ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
  from the interrupt in -rt is a bad thing. Here we simply raise the
  softirq if there's irq work to do. This too boots on my i7 ]

After trying hard to figure out why my i7 box was locking up with the
new active_timers code, that does not run the timer softirq if there
are no active timers, I took an extra look at the softirq handler and
noticed that it doesn't just run timer softirqs, it also runs irq work.

This was the bug that was locking up the system. It wasn't missing a
timer, it was missing irq work. By always doing the irq work callbacks,
the system boots fine.

No need to check for defined(CONFIG_IRQ_WORK). When that's not set the
irq_work_needs_cpu() is a static inline that returns false.

Signed-off-by: Steven Rostedt rost...@goodmis.org

diff --git a/kernel/timer.c b/kernel/timer.c
index 46467be..c01a0d2 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1464,8 +1464,13 @@ void run_local_timers(void)
raise_softirq(TIMER_SOFTIRQ);
return;
}
-   if (!base-active_timers)
-   goto out;
+   if (!base-active_timers) {
+#ifdef CONFIG_PREEMPT_RT_FULL
+   /* On RT, irq work runs from softirq */
+   if (!irq_work_needs_cpu())
+#endif
+   goto out;
+   }
 
/* Check whether the next pending timer has expired */
if (time_before_eq(base-next_timer, jiffies))
--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Sebastian Andrzej Siewior
* Steven Rostedt | 2014-01-24 15:09:33 [-0500]:

[ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
  from the interrupt in -rt is a bad thing. Here we simply raise the
  softirq if there's irq work to do. This too boots on my i7 ]

It is okay in general because most of the users should not run in bare
interrupt context. The only exception here is the nohz_full_kick_work
thing.

After trying hard to figure out why my i7 box was locking up with the
new active_timers code, that does not run the timer softirq if there
are no active timers, I took an extra look at the softirq handler and
noticed that it doesn't just run timer softirqs, it also runs irq work.

This was the bug that was locking up the system. It wasn't missing a
timer, it was missing irq work. By always doing the irq work callbacks,
the system boots fine.

No need to check for defined(CONFIG_IRQ_WORK). When that's not set the
irq_work_needs_cpu() is a static inline that returns false.

Signed-off-by: Steven Rostedt rost...@goodmis.org

Thank you Steven, this makes sense.

Sebastian
--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Steven Rostedt
On Fri, 24 Jan 2014 21:20:39 +0100
Sebastian Andrzej Siewior bige...@linutronix.de wrote:

 * Steven Rostedt | 2014-01-24 15:09:33 [-0500]:
 
 [ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
   from the interrupt in -rt is a bad thing. Here we simply raise the
   softirq if there's irq work to do. This too boots on my i7 ]
 
 It is okay in general because most of the users should not run in bare
 interrupt context. The only exception here is the nohz_full_kick_work
 thing.
 

I know we discussed this on IRC, but I wanted to publicly state that
the missing irq work callback was the RCU's rsp_wakeup() function.

-- 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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Sebastian Andrzej Siewior
On 01/24/2014 09:35 PM, Steven Rostedt wrote:
 
 I know we discussed this on IRC, but I wanted to publicly state that
 the missing irq work callback was the RCU's rsp_wakeup() function.

Let me add that part to that commit message since I can't find it.

 
 -- Steve

Sebastian

--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Paul E. McKenney
On Fri, Jan 24, 2014 at 03:35:42PM -0500, Steven Rostedt wrote:
 On Fri, 24 Jan 2014 21:20:39 +0100
 Sebastian Andrzej Siewior bige...@linutronix.de wrote:
 
  * Steven Rostedt | 2014-01-24 15:09:33 [-0500]:
  
  [ Talking with Sebastian on IRC, it seems that doing the irq_work_run()
from the interrupt in -rt is a bad thing. Here we simply raise the
softirq if there's irq work to do. This too boots on my i7 ]
  
  It is okay in general because most of the users should not run in bare
  interrupt context. The only exception here is the nohz_full_kick_work
  thing.
 
 I know we discussed this on IRC, but I wanted to publicly state that
 the missing irq work callback was the RCU's rsp_wakeup() function.

Failing to invoke rsp_wakeup() when it was needed could potentially
stop RCU grace periods from happening, so having rsp_wakeup() happen
when it is needed is pretty important...

But I would guess that you knew that already.  ;-)

Thanx, Paul

--
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 RT v2] timer: Raise softirq if there's irq_work

2014-01-24 Thread Steven Rostedt
On Fri, 24 Jan 2014 16:19:36 -0800
Paul E. McKenney paul...@linux.vnet.ibm.com wrote:
 
 Failing to invoke rsp_wakeup() when it was needed could potentially
 stop RCU grace periods from happening, so having rsp_wakeup() happen
 when it is needed is pretty important...
 
 But I would guess that you knew that already.  ;-)

Yep, I did. But it's always nice to hear confirmation, which is why I
Cc'd you ;-)

-- 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/