Re: [PATCH 2/6] irq_work: Implement remote queueing

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Stephen Warren]

On 06/10/2014 09:15 AM, Frederic Weisbecker wrote:
> irq work currently only supports local callbacks. However its code
> is mostly ready to run remote callbacks and we have some potential user.
> 
> The full nohz subsystem currently open codes its own remote irq work
> on top of the scheduler ipi when it wants a CPU to reevaluate its next
> tick. However this ad hoc solution bloats the scheduler IPI.
> 
> Lets just extend the irq work subsystem to support remote queuing on top
> of the generic SMP IPI to handle this kind of user. This shouldn't add
> noticeable overhead.

I'm running next-20140624 on an ARM system, and this patch causes CPU
hot(un)plug to Oops for me; the following fires:

void irq_work_run(void)
{
	BUG_ON(!in_irq());

I found that Linus's master (8b8f5d971584 "Merge tag 'compress-3.16-rc3'
of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core")
works fine. I found that this commit inside the tip(?) tree works fine
(478850160636 "irq_work: Implement remote queueing"). However, if I
merge the two together, I hit that BUG_ON.

I think the issue is:

This commit adds a call from
generic_smp_call_function_single_interrupt() to irq_work_run().

Srivatsa's patch adds a call from hotplug_cfd() to
flush_smp_call_function_queue() to, which I imagine happens in
non-interrupt context. Note that this patch moves most of the body of
generic_smp_call_function_single_interrupt() into
flush_smp_call_function_queue().

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Stephen Warren]

On 06/24/2014 02:33 PM, Stephen Warren wrote:
> On 06/10/2014 09:15 AM, Frederic Weisbecker wrote:
>> irq work currently only supports local callbacks. However its code
>> is mostly ready to run remote callbacks and we have some potential user.
>>
>> The full nohz subsystem currently open codes its own remote irq work
>> on top of the scheduler ipi when it wants a CPU to reevaluate its next
>> tick. However this ad hoc solution bloats the scheduler IPI.
>>
>> Lets just extend the irq work subsystem to support remote queuing on top
>> of the generic SMP IPI to handle this kind of user. This shouldn't add
>> noticeable overhead.
> 
> I'm running next-20140624 on an ARM system, and this patch causes CPU
> hot(un)plug to Oops for me; the following fires:
> 
> void irq_work_run(void)
> {
> 	BUG_ON(!in_irq());
> 
> I found that Linus's master (8b8f5d971584 "Merge tag 'compress-3.16-rc3'
> of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core")
> works fine. I found that this commit inside the tip(?) tree works fine
> (478850160636 "irq_work: Implement remote queueing"). However, if I
> merge the two together, I hit that BUG_ON.

I forgot to mention that the conflicting commit from Linus' tree is
8d056c48e486 "CPU hotplug, smp: flush any pending IPI callbacks before
CPU offline".

> I think the issue is:
> 
> This commit adds a call from
> generic_smp_call_function_single_interrupt() to irq_work_run().
> 
> Srivatsa's patch adds a call from hotplug_cfd() to
> flush_smp_call_function_queue() to, which I imagine happens in
> non-interrupt context. Note that this patch moves most of the body of
> generic_smp_call_function_single_interrupt() into
> flush_smp_call_function_queue().

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

On Tue, Jun 24, 2014 at 02:33:41PM -0600, Stephen Warren wrote:
> On 06/10/2014 09:15 AM, Frederic Weisbecker wrote:
> > irq work currently only supports local callbacks. However its code
> > is mostly ready to run remote callbacks and we have some potential user.
> > 
> > The full nohz subsystem currently open codes its own remote irq work
> > on top of the scheduler ipi when it wants a CPU to reevaluate its next
> > tick. However this ad hoc solution bloats the scheduler IPI.
> > 
> > Lets just extend the irq work subsystem to support remote queuing on top
> > of the generic SMP IPI to handle this kind of user. This shouldn't add
> > noticeable overhead.
> 
> I'm running next-20140624 on an ARM system, and this patch causes CPU
> hot(un)plug to Oops for me; the following fires:
> 
> void irq_work_run(void)
> {
> 	BUG_ON(!in_irq());
> 
> I found that Linus's master (8b8f5d971584 "Merge tag 'compress-3.16-rc3'
> of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core")
> works fine. I found that this commit inside the tip(?) tree works fine
> (478850160636 "irq_work: Implement remote queueing"). However, if I
> merge the two together, I hit that BUG_ON.
> 
> I think the issue is:
> 
> This commit adds a call from
> generic_smp_call_function_single_interrupt() to irq_work_run().
> 
> Srivatsa's patch adds a call from hotplug_cfd() to
> flush_smp_call_function_queue() to, which I imagine happens in
> non-interrupt context. Note that this patch moves most of the body of
> generic_smp_call_function_single_interrupt() into
> flush_smp_call_function_queue().

Right you are.. I think I'll just remove the BUG_ON(), Frederic?

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]

On Wed, Jun 25, 2014 at 07:12:34AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 24, 2014 at 02:33:41PM -0600, Stephen Warren wrote:
> > On 06/10/2014 09:15 AM, Frederic Weisbecker wrote:
> > > irq work currently only supports local callbacks. However its code
> > > is mostly ready to run remote callbacks and we have some potential user.
> > > 
> > > The full nohz subsystem currently open codes its own remote irq work
> > > on top of the scheduler ipi when it wants a CPU to reevaluate its next
> > > tick. However this ad hoc solution bloats the scheduler IPI.
> > > 
> > > Lets just extend the irq work subsystem to support remote queuing on top
> > > of the generic SMP IPI to handle this kind of user. This shouldn't add
> > > noticeable overhead.
> > 
> > I'm running next-20140624 on an ARM system, and this patch causes CPU
> > hot(un)plug to Oops for me; the following fires:
> > 
> > void irq_work_run(void)
> > {
> > 	BUG_ON(!in_irq());
> > 
> > I found that Linus's master (8b8f5d971584 "Merge tag 'compress-3.16-rc3'
> > of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core")
> > works fine. I found that this commit inside the tip(?) tree works fine
> > (478850160636 "irq_work: Implement remote queueing"). However, if I
> > merge the two together, I hit that BUG_ON.
> > 
> > I think the issue is:
> > 
> > This commit adds a call from
> > generic_smp_call_function_single_interrupt() to irq_work_run().
> > 
> > Srivatsa's patch adds a call from hotplug_cfd() to
> > flush_smp_call_function_queue() to, which I imagine happens in
> > non-interrupt context. Note that this patch moves most of the body of
> > generic_smp_call_function_single_interrupt() into
> > flush_smp_call_function_queue().
> 
> Right you are.. I think I'll just remove the BUG_ON(), Frederic?

Something a little so like:

---
Subject: irq_work: Remove BUG_ON in irq_work_run_list()
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed Jun 25 07:13:07 CEST 2014

Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
pending IPI callbacks before CPU offline"), which ends up calling
hotplug_cfd()->flush_smp_call_function_queue()->run_irq_work(), which
is not from IRQ context.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Reported-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 kernel/irq_work.c |    2 --
 1 file changed, 2 deletions(-)

--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -130,8 +130,6 @@ static void irq_work_run_list(struct lli
 	struct irq_work *work;
 	struct llist_node *llnode;
 
-	BUG_ON(!irqs_disabled());
-
 	if (llist_empty(list))
 		return;
 

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Srivatsa S. Bhat]

On 06/25/2014 10:47 AM, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 07:12:34AM +0200, Peter Zijlstra wrote:
>> On Tue, Jun 24, 2014 at 02:33:41PM -0600, Stephen Warren wrote:
>>> On 06/10/2014 09:15 AM, Frederic Weisbecker wrote:
>>>> irq work currently only supports local callbacks. However its code
>>>> is mostly ready to run remote callbacks and we have some potential user.

[...]

>> Right you are.. I think I'll just remove the BUG_ON(), Frederic?
> 
> Something a little so like:
> 
> ---
> Subject: irq_work: Remove BUG_ON in irq_work_run_list()

I think this should be irq_work_run(), see below...

> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed Jun 25 07:13:07 CEST 2014
> 
> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
> pending IPI callbacks before CPU offline"), which ends up calling
> hotplug_cfd()->flush_smp_call_function_queue()->run_irq_work(), which


s/run_irq_work/irq_work_run


> is not from IRQ context.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Reported-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/irq_work.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -130,8 +130,6 @@ static void irq_work_run_list(struct lli
>  	struct irq_work *work;
>  	struct llist_node *llnode;
>  
> -	BUG_ON(!irqs_disabled());
> -

I don't think irqs_disabled() is the problematic condition, since
hotplug_cfg() invokes irq_work_run() from CPU_DYING context (which has
irqs disabled). I guess you meant to remove the in_irq() check inside
irq_work_run() instead?

Regards,
Srivatsa S. Bhat

>  	if (llist_empty(list))
>  		return;
>  
> 

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Peter Zijlstra]

On Wed, Jun 25, 2014 at 12:07:05PM +0530, Srivatsa S. Bhat wrote:
> I don't think irqs_disabled() is the problematic condition, since
> hotplug_cfg() invokes irq_work_run() from CPU_DYING context (which has
> irqs disabled). I guess you meant to remove the in_irq() check inside
> irq_work_run() instead?

Yes, clearly I should not get up at 6am.. :-) Let me go do a new one.

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Peter Zijlstra]

On Wed, Jun 25, 2014 at 11:36:57AM +0200, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 12:07:05PM +0530, Srivatsa S. Bhat wrote:
> > I don't think irqs_disabled() is the problematic condition, since
> > hotplug_cfg() invokes irq_work_run() from CPU_DYING context (which has
> > irqs disabled). I guess you meant to remove the in_irq() check inside
> > irq_work_run() instead?
> 
> Yes, clearly I should not get up at 6am.. :-) Let me go do a new one.

---
Subject: irq_work: Remove BUG_ON in irq_work_run()
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed Jun 25 07:13:07 CEST 2014

Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
pending IPI callbacks before CPU offline"), which ends up calling
hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
is not from IRQ context.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Reported-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agipuf@git.kernel.org
---
 kernel/irq_work.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

Index: linux-2.6/kernel/irq_work.c
===================================================================
--- linux-2.6.orig/kernel/irq_work.c
+++ linux-2.6/kernel/irq_work.c
@@ -160,21 +160,11 @@ static void irq_work_run_list(struct lli
 	}
 }
 
-static void __irq_work_run(void)
+static void irq_work_run(void)
 {
 	irq_work_run_list(&__get_cpu_var(raised_list));
 	irq_work_run_list(&__get_cpu_var(lazy_list));
 }
-
-/*
- * Run the irq_work entries on this cpu. Requires to be ran from hardirq
- * context with local IRQs disabled.
- */
-void irq_work_run(void)
-{
-	BUG_ON(!in_irq());
-	__irq_work_run();
-}
 EXPORT_SYMBOL_GPL(irq_work_run);
 
 /*

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Srivatsa S. Bhat]

On 06/25/2014 03:09 PM, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 11:36:57AM +0200, Peter Zijlstra wrote:
>> On Wed, Jun 25, 2014 at 12:07:05PM +0530, Srivatsa S. Bhat wrote:
>>> I don't think irqs_disabled() is the problematic condition, since
>>> hotplug_cfg() invokes irq_work_run() from CPU_DYING context (which has
>>> irqs disabled). I guess you meant to remove the in_irq() check inside
>>> irq_work_run() instead?
>>
>> Yes, clearly I should not get up at 6am.. :-) Let me go do a new one.
> 
> ---
> Subject: irq_work: Remove BUG_ON in irq_work_run()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed Jun 25 07:13:07 CEST 2014
> 
> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
> pending IPI callbacks before CPU offline"), which ends up calling
> hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
> is not from IRQ context.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Reported-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agipuf@git.kernel.org
> ---
>  kernel/irq_work.c |   12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> Index: linux-2.6/kernel/irq_work.c
> ===================================================================
> --- linux-2.6.orig/kernel/irq_work.c
> +++ linux-2.6/kernel/irq_work.c
> @@ -160,21 +160,11 @@ static void irq_work_run_list(struct lli
>  	}
>  }
> 
> -static void __irq_work_run(void)

Hmm, irq_work_cpu_notify() calls __irq_work_run() in the CPU_DYING
phase, to by-pass BUG_ON(!in_irq()). How about doing the same thing
from hotplug_cfd() as well?

> +static void irq_work_run(void)
>  {
>  	irq_work_run_list(&__get_cpu_var(raised_list));
>  	irq_work_run_list(&__get_cpu_var(lazy_list));
>  }
> -
> -/*
> - * Run the irq_work entries on this cpu. Requires to be ran from hardirq
> - * context with local IRQs disabled.
> - */
> -void irq_work_run(void)
> -{
> -	BUG_ON(!in_irq());
> -	__irq_work_run();
> -}
>  EXPORT_SYMBOL_GPL(irq_work_run);
> 
>  /*
> 

Regards,
Srivatsa S. Bhat

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Srivatsa S. Bhat]

On 06/25/2014 03:20 PM, Srivatsa S. Bhat wrote:
> On 06/25/2014 03:09 PM, Peter Zijlstra wrote:
>> On Wed, Jun 25, 2014 at 11:36:57AM +0200, Peter Zijlstra wrote:
>>> On Wed, Jun 25, 2014 at 12:07:05PM +0530, Srivatsa S. Bhat wrote:
>>>> I don't think irqs_disabled() is the problematic condition, since
>>>> hotplug_cfg() invokes irq_work_run() from CPU_DYING context (which has
>>>> irqs disabled). I guess you meant to remove the in_irq() check inside
>>>> irq_work_run() instead?
>>>
>>> Yes, clearly I should not get up at 6am.. :-) Let me go do a new one.
>>
>> ---
>> Subject: irq_work: Remove BUG_ON in irq_work_run()
>> From: Peter Zijlstra <peterz@infradead.org>
>> Date: Wed Jun 25 07:13:07 CEST 2014
>>
>> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
>> pending IPI callbacks before CPU offline"), which ends up calling
>> hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
>> is not from IRQ context.
>>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Reported-by: Stephen Warren <swarren@wwwdotorg.org>
>> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
>> Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agipuf@git.kernel.org
>> ---
>>  kernel/irq_work.c |   12 +-----------
>>  1 file changed, 1 insertion(+), 11 deletions(-)
>>
>> Index: linux-2.6/kernel/irq_work.c
>> ===================================================================
>> --- linux-2.6.orig/kernel/irq_work.c
>> +++ linux-2.6/kernel/irq_work.c
>> @@ -160,21 +160,11 @@ static void irq_work_run_list(struct lli
>>  	}
>>  }
>>
>> -static void __irq_work_run(void)
> 
> Hmm, irq_work_cpu_notify() calls __irq_work_run() in the CPU_DYING
> phase, to by-pass BUG_ON(!in_irq()). How about doing the same thing
> from hotplug_cfd() as well?
> 

Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
irq_work_cpu_notify() altogether?

Regards,
Srivatsa S. Bhat

>> +static void irq_work_run(void)
>>  {
>>  	irq_work_run_list(&__get_cpu_var(raised_list));
>>  	irq_work_run_list(&__get_cpu_var(lazy_list));
>>  }
>> -
>> -/*
>> - * Run the irq_work entries on this cpu. Requires to be ran from hardirq
>> - * context with local IRQs disabled.
>> - */
>> -void irq_work_run(void)
>> -{
>> -	BUG_ON(!in_irq());
>> -	__irq_work_run();
>> -}
>>  EXPORT_SYMBOL_GPL(irq_work_run);
>>
>>  /*
>>
> 

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Peter Zijlstra]

On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
> irq_work_cpu_notify() altogether?

Just so...

getting up at 6am and sitting in an airport terminal doesn't seem to
agree with me; any more silly fail here?

---
Subject: irq_work: Remove BUG_ON in irq_work_run()
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed Jun 25 07:13:07 CEST 2014

Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
pending IPI callbacks before CPU offline"), which ends up calling
hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
is not from IRQ context.

And since that already calls irq_work_run() from the hotplug path,
remove our entire hotplug handling.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Reported-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agipuf@git.kernel.org
---
 kernel/irq_work.c |   48 +++++-------------------------------------------
 1 file changed, 5 insertions(+), 43 deletions(-)

Index: linux-2.6/kernel/irq_work.c
===================================================================
--- linux-2.6.orig/kernel/irq_work.c
+++ linux-2.6/kernel/irq_work.c
@@ -160,20 +160,14 @@ static void irq_work_run_list(struct lli
 	}
 }
 
-static void __irq_work_run(void)
-{
-	irq_work_run_list(&__get_cpu_var(raised_list));
-	irq_work_run_list(&__get_cpu_var(lazy_list));
-}
-
 /*
- * Run the irq_work entries on this cpu. Requires to be ran from hardirq
- * context with local IRQs disabled.
+ * hotplug calls this through:
+ *  hotplug_cfs() -> flush_smp_call_function_queue()
  */
-void irq_work_run(void)
+static void irq_work_run(void)
 {
-	BUG_ON(!in_irq());
-	__irq_work_run();
+	irq_work_run_list(&__get_cpu_var(raised_list));
+	irq_work_run_list(&__get_cpu_var(lazy_list));
 }
 EXPORT_SYMBOL_GPL(irq_work_run);
 
@@ -189,35 +183,3 @@ void irq_work_sync(struct irq_work *work
 		cpu_relax();
 }
 EXPORT_SYMBOL_GPL(irq_work_sync);
-
-#ifdef CONFIG_HOTPLUG_CPU
-static int irq_work_cpu_notify(struct notifier_block *self,
-			       unsigned long action, void *hcpu)
-{
-	long cpu = (long)hcpu;
-
-	switch (action) {
-	case CPU_DYING:
-		/* Called from stop_machine */
-		if (WARN_ON_ONCE(cpu != smp_processor_id()))
-			break;
-		__irq_work_run();
-		break;
-	default:
-		break;
-	}
-	return NOTIFY_OK;
-}
-
-static struct notifier_block cpu_notify;
-
-static __init int irq_work_init_cpu_notifier(void)
-{
-	cpu_notify.notifier_call = irq_work_cpu_notify;
-	cpu_notify.priority = 0;
-	register_cpu_notifier(&cpu_notify);
-	return 0;
-}
-device_initcall(irq_work_init_cpu_notifier);
-
-#endif /* CONFIG_HOTPLUG_CPU */

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Srivatsa S. Bhat]

On 06/25/2014 03:49 PM, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
>> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
>> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
>> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
>> irq_work_cpu_notify() altogether?
> 
> Just so...
> 
> getting up at 6am and sitting in an airport terminal doesn't seem to
> agree with me;

Haha :-)

> any more silly fail here?
> 

A few minor nits below..

> ---
> Subject: irq_work: Remove BUG_ON in irq_work_run()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed Jun 25 07:13:07 CEST 2014
> 
> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
> pending IPI callbacks before CPU offline"), which ends up calling
> hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
> is not from IRQ context.
> 
> And since that already calls irq_work_run() from the hotplug path,
> remove our entire hotplug handling.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Reported-by: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agipuf@git.kernel.org
> ---
>  kernel/irq_work.c |   48 +++++-------------------------------------------
>  1 file changed, 5 insertions(+), 43 deletions(-)
> 
> Index: linux-2.6/kernel/irq_work.c
> ===================================================================
> --- linux-2.6.orig/kernel/irq_work.c
> +++ linux-2.6/kernel/irq_work.c
> @@ -160,20 +160,14 @@ static void irq_work_run_list(struct lli
>  	}
>  }
> 
> -static void __irq_work_run(void)
> -{
> -	irq_work_run_list(&__get_cpu_var(raised_list));
> -	irq_work_run_list(&__get_cpu_var(lazy_list));
> -}
> -
>  /*
> - * Run the irq_work entries on this cpu. Requires to be ran from hardirq
> - * context with local IRQs disabled.
> + * hotplug calls this through:
> + *  hotplug_cfs() -> flush_smp_call_function_queue()

s/hotplug_cfs/hotplug_cfd

>   */
> -void irq_work_run(void)
> +static void irq_work_run(void)

s/static//

>  {
> -	BUG_ON(!in_irq());
> -	__irq_work_run();
> +	irq_work_run_list(&__get_cpu_var(raised_list));
> +	irq_work_run_list(&__get_cpu_var(lazy_list));
>  }
>  EXPORT_SYMBOL_GPL(irq_work_run);

With those 2 changes, everything looks good to me.

Regards,
Srivatsa S. Bhat

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Stephen Warren]

On 06/25/2014 04:19 AM, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
>> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
>> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
>> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
>> irq_work_cpu_notify() altogether?
> 
> Just so...
> 
> getting up at 6am and sitting in an airport terminal doesn't seem to
> agree with me; any more silly fail here?
> 
> ---
> Subject: irq_work: Remove BUG_ON in irq_work_run()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed Jun 25 07:13:07 CEST 2014
> 
> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
> pending IPI callbacks before CPU offline"), which ends up calling
> hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
> is not from IRQ context.
> 
> And since that already calls irq_work_run() from the hotplug path,
> remove our entire hotplug handling.

Tested-by: Stephen Warren <swarren@nvidia.com>

[with the s/static// already mentioned in this thread, obviously:-)]

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Peter Zijlstra]

On Wed, Jun 25, 2014 at 10:23:21AM -0600, Stephen Warren wrote:
> On 06/25/2014 04:19 AM, Peter Zijlstra wrote:
> > On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
> >> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
> >> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
> >> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
> >> irq_work_cpu_notify() altogether?
> > 
> > Just so...
> > 
> > getting up at 6am and sitting in an airport terminal doesn't seem to
> > agree with me; any more silly fail here?
> > 
> > ---
> > Subject: irq_work: Remove BUG_ON in irq_work_run()
> > From: Peter Zijlstra <peterz@infradead.org>
> > Date: Wed Jun 25 07:13:07 CEST 2014
> > 
> > Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
> > pending IPI callbacks before CPU offline"), which ends up calling
> > hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
> > is not from IRQ context.
> > 
> > And since that already calls irq_work_run() from the hotplug path,
> > remove our entire hotplug handling.
> 
> Tested-by: Stephen Warren <swarren@nvidia.com>
> 
> [with the s/static// already mentioned in this thread, obviously:-)]

Right; I pushed out a fixed version right before loosing my tubes at the
airport :-)

https://git.kernel.org/cgit/linux/kernel/git/peterz/queue.git/commit/?h=timers/nohz&id=921d8b81281ecdca686369f52165d04fa3505bd7

I've not gotten wu build bot spam on it so it must be good ;-)

In any case, I'll add your tested-by and update later this evening.

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Srivatsa S. Bhat]

On 06/25/2014 10:08 PM, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 10:23:21AM -0600, Stephen Warren wrote:
>> On 06/25/2014 04:19 AM, Peter Zijlstra wrote:
>>> On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
>>>> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
>>>> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
>>>> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
>>>> irq_work_cpu_notify() altogether?
>>>
>>> Just so...
>>>
>>> getting up at 6am and sitting in an airport terminal doesn't seem to
>>> agree with me; any more silly fail here?
>>>
>>> ---
>>> Subject: irq_work: Remove BUG_ON in irq_work_run()
>>> From: Peter Zijlstra <peterz@infradead.org>
>>> Date: Wed Jun 25 07:13:07 CEST 2014
>>>
>>> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
>>> pending IPI callbacks before CPU offline"), which ends up calling
>>> hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
>>> is not from IRQ context.
>>>
>>> And since that already calls irq_work_run() from the hotplug path,
>>> remove our entire hotplug handling.
>>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>>
>> [with the s/static// already mentioned in this thread, obviously:-)]
> 
> Right; I pushed out a fixed version right before loosing my tubes at the
> airport :-)
> 
> https://git.kernel.org/cgit/linux/kernel/git/peterz/queue.git/commit/?h=timers/nohz&id=921d8b81281ecdca686369f52165d04fa3505bd7
> 

This version looks good.

Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Regards,
Srivatsa S. Bhat


> I've not gotten wu build bot spam on it so it must be good ;-)
> 
> In any case, I'll add your tested-by and update later this evening.
> 

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Borislav Petkov]

On Wed, Jun 25, 2014 at 10:27:24PM +0530, Srivatsa S. Bhat wrote:
> > https://git.kernel.org/cgit/linux/kernel/git/peterz/queue.git/commit/?h=timers/nohz&id=921d8b81281ecdca686369f52165d04fa3505bd7

Just hit it here too. Cherry-picking it ontop of latest tip fixes the
issue, thanks.

Tested-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Stephen Warren]

On 06/25/2014 10:23 AM, Stephen Warren wrote:
> On 06/25/2014 04:19 AM, Peter Zijlstra wrote:
>> On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
>>> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
>>> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
>>> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
>>> irq_work_cpu_notify() altogether?
>>
>> Just so...
>>
>> getting up at 6am and sitting in an airport terminal doesn't seem to
>> agree with me; any more silly fail here?
>>
>> ---
>> Subject: irq_work: Remove BUG_ON in irq_work_run()
>> From: Peter Zijlstra <peterz@infradead.org>
>> Date: Wed Jun 25 07:13:07 CEST 2014
>>
>> Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
>> pending IPI callbacks before CPU offline"), which ends up calling
>> hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
>> is not from IRQ context.
>>
>> And since that already calls irq_work_run() from the hotplug path,
>> remove our entire hotplug handling.
> 
> Tested-by: Stephen Warren <swarren@nvidia.com>
> 
> [with the s/static// already mentioned in this thread, obviously:-)]

next-20140701 still seems to fail CPU hotplug. I assume this patch
hasn't yet been applied for some reason?

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Frederic Weisbecker]

On Wed, Jun 25, 2014 at 06:38:20PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 25, 2014 at 10:23:21AM -0600, Stephen Warren wrote:
> > On 06/25/2014 04:19 AM, Peter Zijlstra wrote:
> > > On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
> > >> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
> > >> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
> > >> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
> > >> irq_work_cpu_notify() altogether?
> > > 
> > > Just so...
> > > 
> > > getting up at 6am and sitting in an airport terminal doesn't seem to
> > > agree with me; any more silly fail here?
> > > 
> > > ---
> > > Subject: irq_work: Remove BUG_ON in irq_work_run()
> > > From: Peter Zijlstra <peterz@infradead.org>
> > > Date: Wed Jun 25 07:13:07 CEST 2014
> > > 
> > > Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
> > > pending IPI callbacks before CPU offline"), which ends up calling
> > > hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
> > > is not from IRQ context.
> > > 
> > > And since that already calls irq_work_run() from the hotplug path,
> > > remove our entire hotplug handling.
> > 
> > Tested-by: Stephen Warren <swarren@nvidia.com>
> > 
> > [with the s/static// already mentioned in this thread, obviously:-)]
> 
> Right; I pushed out a fixed version right before loosing my tubes at the
> airport :-)
> 
> https://git.kernel.org/cgit/linux/kernel/git/peterz/queue.git/commit/?h=timers/nohz&id=921d8b81281ecdca686369f52165d04fa3505bd7
> 
> I've not gotten wu build bot spam on it so it must be good ;-)
> 
> In any case, I'll add your tested-by and update later this evening.

Ack! Thanks for fixing this.

In case you're AFK, do you need any help for pushing the patch or so?

Re: [PATCH 2/6] irq_work: Implement remote queueing

[Sachin Kamat]

On Wed, Jun 25, 2014 at 10:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jun 25, 2014 at 10:23:21AM -0600, Stephen Warren wrote:
>> On 06/25/2014 04:19 AM, Peter Zijlstra wrote:
>> > On Wed, Jun 25, 2014 at 03:24:11PM +0530, Srivatsa S. Bhat wrote:
>> >> Wait, that was a stupid idea. hotplug_cfd() already invokes irq_work_run
>> >> indirectly via flush_smp_call_function_queue(). So irq_work_cpu_notify()
>> >> doesn't need to invoke it again, AFAIU. So perhaps we can get rid of
>> >> irq_work_cpu_notify() altogether?
>> >
>> > Just so...
>> >
>> > getting up at 6am and sitting in an airport terminal doesn't seem to
>> > agree with me; any more silly fail here?
>> >
>> > ---
>> > Subject: irq_work: Remove BUG_ON in irq_work_run()
>> > From: Peter Zijlstra <peterz@infradead.org>
>> > Date: Wed Jun 25 07:13:07 CEST 2014
>> >
>> > Because of a collision with 8d056c48e486 ("CPU hotplug, smp: flush any
>> > pending IPI callbacks before CPU offline"), which ends up calling
>> > hotplug_cfd()->flush_smp_call_function_queue()->irq_work_run(), which
>> > is not from IRQ context.
>> >
>> > And since that already calls irq_work_run() from the hotplug path,
>> > remove our entire hotplug handling.
>>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>>
>> [with the s/static// already mentioned in this thread, obviously:-)]
>
> Right; I pushed out a fixed version right before loosing my tubes at the
> airport :-)
>
> https://git.kernel.org/cgit/linux/kernel/git/peterz/queue.git/commit/?h=timers/nohz&id=921d8b81281ecdca686369f52165d04fa3505bd7

This patch fixes boot issue on Exynos5420/5800 based boards with bL
switcher enabled.

FWIW,
Tested-by: Sachin Kamat <sachin.kamat@samsung.com>

-- 
Regards,
Sachin.