Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752537AbbGQB3o (ORCPT ); Thu, 16 Jul 2015 21:29:44 -0400 Received: from mail-bl2on0121.outbound.protection.outlook.com ([65.55.169.121]:9108 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751575AbbGQB3m (ORCPT ); Thu, 16 Jul 2015 21:29:42 -0400 X-Greylist: delayed 67927 seconds by postgrey-1.27 at vger.kernel.org; Thu, 16 Jul 2015 21:29:42 EDT From: Duan Andy To: Krzysztof Kozlowski CC: Kamal Mostafa , "daniel.lezcano@linaro.org" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" , Damian Eppel , "kgene@kernel.org" , "kyungmin.park@samsung.com" , Thomas Gleixner , "m.szyprowski@samsung.com" , "linux-arm-kernel@lists.infradead.org" , "kernel-team@lists.ubuntu.com" Subject: RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier Thread-Topic: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier Thread-Index: AQHQv2ULRJkCJ+tWi0is9V1wrJLW1p3dpClQgAAF6ICAABVGUIAAHBIAgAEFVBA= Date: Fri, 17 Jul 2015 01:29:39 +0000 Message-ID: References: <1437008972-9140-1-git-send-email-kamal@canonical.com> <1437008972-9140-174-git-send-email-kamal@canonical.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: samsung.com; dkim=none (message not signed) header.d=none; x-originating-ip: [199.59.231.64] x-microsoft-exchange-diagnostics: 1;BY2PR03MB379;5:bE5KME69mLJC6UIHjxdFliqinq9X5oyD564g46WjzPw8jRLeNZRFi++h199Ml3f4r5xmaP0hafbmGY06iikIhyZq2aDxDgAjhigpW/cTMNCDptxbb4ez0IIANVViluIVKWizrY8FqiYa9wDd12mZnw==;24:G+/6LSIt0p3dsHXyaDmAQ3Zo0kB5YJk2D1FJu7zoTRlEDXNBpbCw3dfq7MDOPTgTbBXW4FZF2iRBuq8aTB8SX8o+yDwWTGvv/O6E1GuxU6I=;20:RvaJ05q/6uSRWKTWgZZyozFGy4tkpcoupyPei0n+VmpJ61DNmTJA3NyIHpZu24jmmEEMPAQjeb7UGzaBDY4pGw== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB379; by2pr03mb379: X-MS-Exchange-Organization-RulesExecuted x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(5005006)(3002001);SRVR:BY2PR03MB379;BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB379; x-forefront-prvs: 06400060E1 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(377454003)(54534003)(377424004)(93886004)(106116001)(87936001)(189998001)(99286002)(66066001)(76576001)(33656002)(2950100001)(19580405001)(2656002)(575784001)(19580395003)(76176999)(50986999)(54356999)(74316001)(86362001)(122556002)(62966003)(5002640100001)(77156002)(5003600100002)(15975445007)(1720100001)(5001960100002)(77096005)(2900100001)(102836002)(46102003)(110136002)(92566002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY2PR03MB379;H:BY2PR03MB377.namprd03.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Jul 2015 01:29:39.9166 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR03MB379 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t6H1TnCI014482 Content-Length: 8711 Lines: 192 From: Krzysztof Kozlowski Sent: Thursday, July 16, 2015 5:52 PM > To: Duan Fugang-B38611 > Cc: Krzysztof Kozlowski; Kamal Mostafa; daniel.lezcano@linaro.org; linux- > kernel@vger.kernel.org; stable@vger.kernel.org; Damian Eppel; > kgene@kernel.org; kyungmin.park@samsung.com; Thomas Gleixner; > m.szyprowski@samsung.com; linux-arm-kernel@lists.infradead.org; kernel- > team@lists.ubuntu.com > Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid > blocking calls in the cpu hotplug notifier > > 2015-07-16 17:14 GMT+09:00 Duan Andy : > > From: Krzysztof Kozlowski Sent: Thursday, > > July 16, 2015 2:56 PM > >> To: Duan Fugang-B38611 > >> Cc: Kamal Mostafa; linux-kernel@vger.kernel.org; > >> stable@vger.kernel.org; kernel-team@lists.ubuntu.com; > >> daniel.lezcano@linaro.org; Damian Eppel; kyungmin.park@samsung.com; > >> kgene@kernel.org; Thomas Gleixner; linux-arm- > >> kernel@lists.infradead.org; m.szyprowski@samsung.com > >> Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: > >> Avoid blocking calls in the cpu hotplug notifier > >> > >> 2015-07-16 15:37 GMT+09:00 Duan Andy : > >> > From: Kamal Mostafa Sent: Thursday, July 16, > >> > 2015 9:08 AM > >> >> To: linux-kernel@vger.kernel.org; stable@vger.kernel.org; kernel- > >> >> team@lists.ubuntu.com > >> >> Cc: Kamal Mostafa; daniel.lezcano@linaro.org; > >> >> kyungmin.park@samsung.com; Damian Eppel; kgene@kernel.org; Thomas > >> >> Gleixner; linux-arm- kernel@lists.infradead.org; > >> >> m.szyprowski@samsung.com > >> >> Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid > >> >> blocking calls in the cpu hotplug notifier > >> >> > >> >> 3.19.8-ckt4 -stable review patch. If anyone has any objections, > >> >> please let me know. > >> >> > >> >> ------------------ > >> >> > >> >> From: Damian Eppel > >> >> > >> >> commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream. > >> >> > >> >> Whilst testing cpu hotplug events on kernel configured with > >> >> DEBUG_PREEMPT and DEBUG_ATOMIC_SLEEP we get following BUG message, > >> >> caused by calling > >> >> request_irq() and free_irq() in the context of hotplug > >> >> notification (which is in this case atomic context). > >> >> > >> >> [ 40.785859] CPU1: Software reset > >> >> [ 40.786660] BUG: sleeping function called from invalid context > at > >> >> mm/slub.c:1241 > >> >> [ 40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: > >> >> swapper/1 > >> >> [ 40.786678] Preemption disabled at:[< (null)>] (null) > >> >> [ 40.786681] > >> >> [ 40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0- > rc4- > >> >> 00024-g7dca860 #36 > >> >> [ 40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > >> >> [ 40.786728] [] (unwind_backtrace) from [] > >> >> (show_stack+0x10/0x14) > >> >> [ 40.786747] [] (show_stack) from [] > >> >> (dump_stack+0x70/0xbc) > >> >> [ 40.786767] [] (dump_stack) from [] > >> >> (kmem_cache_alloc+0xd8/0x170) > >> >> [ 40.786785] [] (kmem_cache_alloc) from [] > >> >> (request_threaded_irq+0x64/0x128) > >> >> [ 40.786804] [] (request_threaded_irq) from [] > >> >> (exynos4_local_timer_setup+0xc0/0x13c) > >> >> [ 40.786820] [] (exynos4_local_timer_setup) from > >> [] > >> >> (exynos4_mct_cpu_notify+0x30/0xa8) > >> >> [ 40.786838] [] (exynos4_mct_cpu_notify) from > [] > >> >> (notifier_call_chain+0x44/0x84) > >> >> [ 40.786857] [] (notifier_call_chain) from [] > >> >> (__cpu_notify+0x28/0x44) > >> >> [ 40.786873] [] (__cpu_notify) from [] > >> >> (secondary_start_kernel+0xec/0x150) > >> >> [ 40.786886] [] (secondary_start_kernel) from > [<40008764>] > >> >> (0x40008764) > >> >> > >> >> Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING > >> >> notifications which run on the hotplugged cpu with interrupts and > >> >> preemption disabled. > >> >> > >> >> To avoid the issue, request the interrupts for all possible cpus > >> >> in the boot code. The interrupts are marked NO_AUTOENABLE to avoid > >> >> a racy > >> >> request_irq/disable_irq() sequence. The flag prevents the > >> >> request_irq() code from enabling the interrupt immediately. > >> >> > >> >> The interrupt is then enabled in the CPU_STARTING notifier of the > >> >> hotplugged cpu and again disabled with disable_irq_nosync() in the > >> >> CPU_DYING notifier. > >> >> > >> >> [ tglx: Massaged changelog to match the patch ] > >> >> > >> >> Fixes: 7114cd749a12 ("clocksource: exynos_mct: use > >> >> (request/free)_irq calls for local timer registration") > >> >> Reported-by: Krzysztof Kozlowski > >> >> Reviewed-by: Krzysztof Kozlowski > >> >> Tested-by: Krzysztof Kozlowski > >> >> Tested-by: Marcin Jabrzyk > >> >> Signed-off-by: Damian Eppel > >> >> Cc: m.szyprowski@samsung.com > >> >> Cc: kyungmin.park@samsung.com > >> >> Cc: daniel.lezcano@linaro.org > >> >> Cc: kgene@kernel.org > >> >> Cc: linux-arm-kernel@lists.infradead.org > >> >> Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email- > >> >> d.eppel@samsung.com > >> >> Signed-off-by: Thomas Gleixner > >> >> Signed-off-by: Kamal Mostafa > >> >> --- > >> >> drivers/clocksource/exynos_mct.c | 43 > >> >> ++++++++++++++++++++++++++++------ > >> >> ------ > >> >> 1 file changed, 30 insertions(+), 13 deletions(-) > >> >> > >> >> diff --git a/drivers/clocksource/exynos_mct.c > >> >> b/drivers/clocksource/exynos_mct.c > >> >> index 83564c9..c844616 100644 > >> >> --- a/drivers/clocksource/exynos_mct.c > >> >> +++ b/drivers/clocksource/exynos_mct.c > >> >> @@ -466,15 +466,12 @@ static int exynos4_local_timer_setup(struct > >> >> clock_event_device *evt) > >> >> exynos4_mct_write(TICK_BASE_CNT, mevt->base + > >> >> MCT_L_TCNTB_OFFSET); > >> >> > >> >> if (mct_int_type == MCT_INT_SPI) { > >> >> - evt->irq = mct_irqs[MCT_L0_IRQ + cpu]; > >> >> - if (request_irq(evt->irq, exynos4_mct_tick_isr, > >> >> - IRQF_TIMER | IRQF_NOBALANCING, > >> >> - evt->name, mevt)) { > >> >> - pr_err("exynos-mct: cannot register IRQ %d\n", > >> >> - evt->irq); > >> >> + > >> >> + if (evt->irq == -1) > >> >> return -EIO; > >> >> - } > >> >> - irq_force_affinity(mct_irqs[MCT_L0_IRQ + cpu], > >> >> cpumask_of(cpu)); > >> >> + > >> >> + irq_force_affinity(evt->irq, cpumask_of(cpu)); > >> >> + enable_irq(evt->irq); > >> >> } else { > >> >> enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0); > >> > > >> > In here, why not use enable_percpu_irq(evt->irq) ? > >> > > >> >> } > >> >> @@ -487,10 +484,12 @@ static int exynos4_local_timer_setup(struct > >> >> clock_event_device *evt) static void > >> >> exynos4_local_timer_stop(struct clock_event_device *evt) { > >> >> evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt); > >> >> - if (mct_int_type == MCT_INT_SPI) > >> >> - free_irq(evt->irq, this_cpu_ptr(&percpu_mct_tick)); > >> >> - else > >> >> + if (mct_int_type == MCT_INT_SPI) { > >> >> + if (evt->irq != -1) > >> >> + disable_irq_nosync(evt->irq); > >> >> + } else { > >> >> disable_percpu_irq(mct_irqs[MCT_L0_IRQ]); > >> > > >> > In here, why not use disable_percpu_irq(evt->irq) ? > >> > >> You know this is just a semi-automatic stable backport and this was > >> already merged to mainline? > >> > >> Best regards, > >> Krzysztof > > > > Yes, I know. Do you know the reason why not enable other cpus irq ? > > I want to confirm whether it is right after below change ? > > enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0); -> > > enable_percpu_irq(evt->irq, 0); > > disable_percpu_irq(mct_irqs[MCT_L0_IRQ]) -> > > disable_percpu_irq(evt->irq) > > This is a backport so are there any objections about this patch? Do you > think the patch is invalid so it should not be backported to stable? > No, I just have some questions about the original code design. Your patch has no problem. Regards, Andy ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?