From: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> To: "Michał Mirosław" <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Subject: Re: [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Date: Thu, 20 Jul 2017 13:45:12 +0100 [thread overview] Message-ID: <378bf911-147e-f800-2de4-591e160528f0@nvidia.com> (raw) In-Reply-To: <0a0600cdbcc9a71134105043c3e2ace84bab7c5a.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> On 20/07/17 01:29, Michał Mirosław wrote: > This removes unnecessary lock causing following BUG splat: > > BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747 > in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0 > no locks held by swapper/0/0. > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1 > Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) > [<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c) > [<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98) > [<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c) > [<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0) > [<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c) > [<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4) > [<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c) > [<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114) > [<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c) > [<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204) > [<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24) > [<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8) > > Signed-off-by: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> > --- > drivers/soc/tegra/pmc.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > index e233dd5dcab3..3e6ec9fdba41 100644 > --- a/drivers/soc/tegra/pmc.c > +++ b/drivers/soc/tegra/pmc.c > @@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id) > if (!tegra_powergate_is_valid(id)) > return -EINVAL; > > - mutex_lock(&pmc->powergates_lock); > status = tegra_powergate_state(id); > - mutex_unlock(&pmc->powergates_lock); > > return status; > } Thanks for the fix. However, I would prefer that we fix this the following way ... diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c index a2d163f759b4..546d2b121c39 100644 --- a/drivers/clk/tegra/clk-tegra30.c +++ b/drivers/clk/tegra/clk-tegra30.c @@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void) cpu_rst_status = readl(clk_base + TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS); - cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) || - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) || - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3); + cpu_pwr_status = tegra_pmc_cpu_is_powered(1) || + tegra_pmc_cpu_is_powered(2) || + tegra_pmc_cpu_is_powered(3); if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status) return false; diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index e233dd5dcab3..f61371ea3fe0 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid) if (id < 0) return false; - return tegra_powergate_is_powered(id); + return tegra_powergate_state(id); } Could you try the above and make sure that this works? I would prefer to leave the mutex in tegra_powergate_is_powered() so it could be used by any driver. Or maybe we could even get rid of tegra_powergate_is_powered() if we don't really need it. Cheers Jon -- nvpublic
WARNING: multiple messages have this Message-ID (diff)
From: jonathanh@nvidia.com (Jon Hunter) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Date: Thu, 20 Jul 2017 13:45:12 +0100 [thread overview] Message-ID: <378bf911-147e-f800-2de4-591e160528f0@nvidia.com> (raw) In-Reply-To: <0a0600cdbcc9a71134105043c3e2ace84bab7c5a.1500510157.git.mirq-linux@rere.qmqm.pl> On 20/07/17 01:29, Micha? Miros?aw wrote: > This removes unnecessary lock causing following BUG splat: > > BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747 > in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0 > no locks held by swapper/0/0. > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1 > Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) > [<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c) > [<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98) > [<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c) > [<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0) > [<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c) > [<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4) > [<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c) > [<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114) > [<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c) > [<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204) > [<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24) > [<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8) > > Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl> > --- > drivers/soc/tegra/pmc.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > index e233dd5dcab3..3e6ec9fdba41 100644 > --- a/drivers/soc/tegra/pmc.c > +++ b/drivers/soc/tegra/pmc.c > @@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id) > if (!tegra_powergate_is_valid(id)) > return -EINVAL; > > - mutex_lock(&pmc->powergates_lock); > status = tegra_powergate_state(id); > - mutex_unlock(&pmc->powergates_lock); > > return status; > } Thanks for the fix. However, I would prefer that we fix this the following way ... diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c index a2d163f759b4..546d2b121c39 100644 --- a/drivers/clk/tegra/clk-tegra30.c +++ b/drivers/clk/tegra/clk-tegra30.c @@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void) cpu_rst_status = readl(clk_base + TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS); - cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) || - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) || - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3); + cpu_pwr_status = tegra_pmc_cpu_is_powered(1) || + tegra_pmc_cpu_is_powered(2) || + tegra_pmc_cpu_is_powered(3); if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status) return false; diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index e233dd5dcab3..f61371ea3fe0 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid) if (id < 0) return false; - return tegra_powergate_is_powered(id); + return tegra_powergate_state(id); } Could you try the above and make sure that this works? I would prefer to leave the mutex in tegra_powergate_is_powered() so it could be used by any driver. Or maybe we could even get rid of tegra_powergate_is_powered() if we don't really need it. Cheers Jon -- nvpublic
next prev parent reply other threads:[~2017-07-20 12:45 UTC|newest] Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <cover.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-07-20 0:29 ` [PATCH 2/9] arm: cache-l2x0: remove duplicate warning Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław 2017-07-20 0:29 ` [PATCH 3/9] arm: cache-l2x0: share l2x0_base Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <eb104cfdf44ef9bf7b4f271555e1496d2bf781bf.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-10-05 16:48 ` Mark Rutland 2017-10-05 16:48 ` Mark Rutland 2017-12-19 23:10 ` Russell King - ARM Linux 2017-12-19 23:10 ` Russell King - ARM Linux 2017-07-20 0:29 ` [PATCH 1/9] ARM: enable secure platform-only erratas Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <77ce738c15b992a92bee3a18e5468342fb2dc5ab.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-10-05 15:53 ` Dmitry Osipenko 2017-10-05 15:53 ` Dmitry Osipenko 2017-10-05 18:16 ` Dmitry Osipenko 2017-10-05 18:16 ` Dmitry Osipenko [not found] ` <bdff72b9-8ebd-a426-b27b-fe055d45cfb1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-12-19 23:28 ` Russell King - ARM Linux 2017-12-19 23:28 ` Russell King - ARM Linux [not found] ` <20171219232810.GI10595-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 2017-12-20 12:49 ` Dmitry Osipenko 2017-12-20 12:49 ` Dmitry Osipenko 2017-07-20 0:29 ` [PATCH 5/9] ARM: trusted_foundations: announce firmware version Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław 2017-07-20 0:29 ` [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <1d12ea86cca40749731a594afc165830c0b2463d.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-10-05 16:24 ` Dmitry Osipenko 2017-10-05 16:24 ` Dmitry Osipenko 2017-12-19 18:56 ` Dmitry Osipenko 2017-12-19 18:56 ` Dmitry Osipenko 2017-07-20 0:29 ` [PATCH 8/9] ARM: tegra: avoid touching Secure registers in reset handler Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław 2017-07-20 0:29 ` [PATCH 6/9] ARM: init: update secondary_data register documentation Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <a6e5e735ebae32a433e501188a86082efc8b0b52.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-12-19 23:16 ` Russell King - ARM Linux 2017-12-19 23:16 ` Russell King - ARM Linux 2017-07-20 0:29 ` [PATCH 7/9] ARM: tegra: enable cache via TF Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <6a164b2270a3e996c083e94bf5b1e27028c1135e.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-12-19 19:07 ` Dmitry Osipenko 2017-12-19 19:07 ` Dmitry Osipenko 2017-12-19 23:21 ` Russell King - ARM Linux 2017-12-19 23:21 ` Russell King - ARM Linux 2017-07-20 0:29 ` [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Michał Mirosław 2017-07-20 0:29 ` Michał Mirosław [not found] ` <0a0600cdbcc9a71134105043c3e2ace84bab7c5a.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-07-20 12:45 ` Jon Hunter [this message] 2017-07-20 12:45 ` Jon Hunter [not found] ` <378bf911-147e-f800-2de4-591e160528f0-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2017-07-20 16:28 ` Michał Mirosław 2017-07-20 16:28 ` Michał Mirosław [not found] ` <20170720162852.rh565t5as2tbe6np-cHozx32mtrEEUmgCuDUIdw@public.gmane.org> 2017-07-21 8:15 ` Jon Hunter 2017-07-21 8:15 ` Jon Hunter 2017-07-20 7:48 ` [PATCH 0/9] Bringing Asus TF300T support to mainline Mikko Perttunen 2017-07-20 7:48 ` Mikko Perttunen [not found] ` <a4149270-561a-4190-4c4e-c164bd91446e-/1wQRMveznE@public.gmane.org> 2017-07-20 15:07 ` Michał Mirosław 2017-07-20 15:07 ` Michał Mirosław 2017-10-04 21:25 ` Michał Mirosław 2017-10-04 21:25 ` Michał Mirosław [not found] ` <20171004212534.uosfz6757bbds2c5-cHozx32mtrEEUmgCuDUIdw@public.gmane.org> 2017-10-05 15:52 ` Dmitry Osipenko 2017-10-05 15:52 ` Dmitry Osipenko [not found] ` <dcbbc541-60c1-8af5-25ea-927629447a55-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-10-05 16:07 ` Dmitry Osipenko 2017-10-05 16:07 ` Dmitry Osipenko -- strict thread matches above, loose matches on Subject: below -- 2017-07-20 0:14 Michał Mirosław [not found] ` <cover.1500509346.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org> 2017-07-20 0:14 ` [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Michał Mirosław
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=378bf911-147e-f800-2de4-591e160528f0@nvidia.com \ --to=jonathanh-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.