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

2014-07-03 Thread Sachin Kamat
On Wed, Jun 25, 2014 at 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 
>> > 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 
>>
>> [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 

-- 
Regards,
Sachin.
--
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 2/6] irq_work: Implement remote queueing

2014-07-03 Thread 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 
> > > 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 
> > 
> > [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?
--
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 2/6] irq_work: Implement remote queueing

2014-07-01 Thread 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 
>> 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 
> 
> [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?
--
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 2/6] irq_work: Implement remote queueing

2014-06-28 Thread 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 

-- 
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
>>> 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 
>>
>> [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 

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

--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
> > 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 
> 
> [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.
--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
> 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 

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

--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
> 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 
> Reported-by: Stephen Warren 
> Signed-off-by: Peter Zijlstra 
> Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agi...@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



--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
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 
Reported-by: Stephen Warren 
Signed-off-by: Peter Zijlstra 
Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agi...@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 */
--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
>> 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 
>> Reported-by: Stephen Warren 
>> Signed-off-by: Peter Zijlstra 
>> Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agi...@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);
>>
>>  /*
>>
> 

--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
> 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 
> Reported-by: Stephen Warren 
> Signed-off-by: Peter Zijlstra 
> Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agi...@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

--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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 
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 
Reported-by: Stephen Warren 
Signed-off-by: Peter Zijlstra 
Link: http://lkml.kernel.org/n/tip-busatzs2gvz4v62258agi...@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);
 
 /*
--
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 2/6] irq_work: Implement remote queueing

2014-06-25 Thread 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.
--
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 2/6] irq_work: Implement remote queueing

2014-06-24 Thread 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 
> 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 
> Reported-by: Stephen Warren 
> Signed-off-by: Peter Zijlstra 
> ---
>  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;
>  
> 

--
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 2/6] irq_work: Implement remote queueing

2014-06-24 Thread Peter Zijlstra
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 
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 
Reported-by: Stephen Warren 
Signed-off-by: Peter Zijlstra 
---
 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;
 


pgprg6uRfCMm1.pgp
Description: PGP signature


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

2014-06-24 Thread Peter Zijlstra
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?


pgpG3f3rkF88E.pgp
Description: PGP signature


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

2014-06-24 Thread 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().
--
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 2/6] irq_work: Implement remote queueing

2014-06-24 Thread 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().
--
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 2/6] irq_work: Implement remote queueing

2014-06-10 Thread Frederic Weisbecker
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.

Suggested-by: Peter Zijlstra 
Acked-by: Peter Zijlstra 
Cc: Andrew Morton 
Cc: Eric Dumazet 
Cc: Ingo Molnar 
Cc: Kevin Hilman 
Cc: Paul E. McKenney 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Viresh Kumar 
Signed-off-by: Frederic Weisbecker 
---
 include/linux/irq_work.h |  5 +
 kernel/irq_work.c| 25 -
 kernel/smp.c |  9 +
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 19ae05d..bf9422c 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -33,6 +33,11 @@ void init_irq_work(struct irq_work *work, void 
(*func)(struct irq_work *))
 #define DEFINE_IRQ_WORK(name, _f) struct irq_work name = { .func = (_f), }
 
 bool irq_work_queue(struct irq_work *work);
+
+#ifdef CONFIG_SMP
+bool irq_work_queue_on(struct irq_work *work, int cpu);
+#endif
+
 void irq_work_run(void);
 void irq_work_sync(struct irq_work *work);
 
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 126f254..4b0a890 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 
@@ -55,12 +56,34 @@ void __weak arch_irq_work_raise(void)
 */
 }
 
+#ifdef CONFIG_SMP
 /*
- * Enqueue the irq_work @entry unless it's already pending
+ * Enqueue the irq_work @work on @cpu unless it's already pending
  * somewhere.
  *
  * Can be re-enqueued while the callback is still in progress.
  */
+bool irq_work_queue_on(struct irq_work *work, int cpu)
+{
+   /* All work should have been flushed before going offline */
+   WARN_ON_ONCE(cpu_is_offline(cpu));
+
+   /* Arch remote IPI send/receive backend aren't NMI safe */
+   WARN_ON_ONCE(in_nmi());
+
+   /* Only queue if not already pending */
+   if (!irq_work_claim(work))
+   return false;
+
+   if (llist_add(&work->llnode, &per_cpu(raised_list, cpu)))
+   arch_send_call_function_single_ipi(cpu);
+
+   return true;
+}
+EXPORT_SYMBOL_GPL(irq_work_queue_on);
+#endif
+
+/* Enqueue the irq work @work on the current CPU */
 bool irq_work_queue(struct irq_work *work)
 {
/* Only queue if not already pending */
diff --git a/kernel/smp.c b/kernel/smp.c
index 06d574e..6e9ff62 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -3,6 +3,7 @@
  *
  * (C) Jens Axboe  2008
  */
+#include 
 #include 
 #include 
 #include 
@@ -198,6 +199,14 @@ void generic_smp_call_function_single_interrupt(void)
csd->func(csd->info);
csd_unlock(csd);
}
+
+   /*
+* Handle irq works queued remotely by irq_work_queue_on().
+* Smp functions above are typically synchronous so they
+* better run first since some other CPUs may be busy waiting
+* for them.
+*/
+   irq_work_run();
 }
 
 /*
-- 
1.8.3.1

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