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