Re: [PATCH] clk: Fix __GFP_FS allocation with irqs disabled
Quoting Stephen Boyd (2014-12-22 11:26:42)
> On 12/22/2014 10:38 AM, Stephen Boyd wrote:
> > On 12/22/2014 03:45 AM, Krzysztof Kozlowski wrote:
> >
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >> index f4963b7d4e17..35079302a650 100644
> >> --- a/drivers/clk/clk.c
> >> +++ b/drivers/clk/clk.c
> >> @@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
> >> struct clk *parent)
> >> {
> >> int i;
> >>
> >> +if (clk->num_parents == 1) {
> >> +if (IS_ERR_OR_NULL(clk->parent))
> >> +clk->parent = __clk_lookup(clk->parent_names[0]);
> >> +return 0;
> >> +}
> >> +
> >> if (!clk->parents) {
> >> clk->parents = kcalloc(clk->num_parents,
> >> sizeof(struct clk *), GFP_KERNEL);
> > This may be a worthwhile optimization, but I wonder why the clk_ops for
> > this clock need to fetch the parent index at all? Which clock are we
> > actually dealing with here?
> >
>
> The clk_set_rate() call should still be fixed, but we can probably do
> this too.
>
> ---8<---
>
> From: Stephen Boyd
> Date: Mon, 22 Dec 2014 11:24:28 -0800
> Subject: [PATCH] clk: Skip fetching index for single parent clocks
>
> We don't need to fetch the parent index for clocks if they only
> have one parent. Doing this also avoid an unnecessary allocation
> for the parent cache.
>
> Signed-off-by: Stephen Boyd
> ---
> drivers/clk/clk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 675f37a7329f..e3a2d36124fd 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1420,7 +1420,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
> unsigned long rate)
> }
>
> /* try finding the new parent index */
> - if (parent) {
> + if (parent && clk->num_parents > 1) {
> p_index = clk_fetch_parent_index(clk, parent);
> if (p_index < 0) {
> pr_debug("%s: clk %s can not be parent of clk %s\n",
Applied to clk-next.
Regards,
Mike
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
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] clk: Fix __GFP_FS allocation with irqs disabled
Quoting Stephen Boyd (2014-12-22 11:26:42)
On 12/22/2014 10:38 AM, Stephen Boyd wrote:
On 12/22/2014 03:45 AM, Krzysztof Kozlowski wrote:
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f4963b7d4e17..35079302a650 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
struct clk *parent)
{
int i;
+if (clk-num_parents == 1) {
+if (IS_ERR_OR_NULL(clk-parent))
+clk-parent = __clk_lookup(clk-parent_names[0]);
+return 0;
+}
+
if (!clk-parents) {
clk-parents = kcalloc(clk-num_parents,
sizeof(struct clk *), GFP_KERNEL);
This may be a worthwhile optimization, but I wonder why the clk_ops for
this clock need to fetch the parent index at all? Which clock are we
actually dealing with here?
The clk_set_rate() call should still be fixed, but we can probably do
this too.
---8---
From: Stephen Boyd sb...@codeaurora.org
Date: Mon, 22 Dec 2014 11:24:28 -0800
Subject: [PATCH] clk: Skip fetching index for single parent clocks
We don't need to fetch the parent index for clocks if they only
have one parent. Doing this also avoid an unnecessary allocation
for the parent cache.
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
drivers/clk/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 675f37a7329f..e3a2d36124fd 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1420,7 +1420,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
unsigned long rate)
}
/* try finding the new parent index */
- if (parent) {
+ if (parent clk-num_parents 1) {
p_index = clk_fetch_parent_index(clk, parent);
if (p_index 0) {
pr_debug(%s: clk %s can not be parent of clk %s\n,
Applied to clk-next.
Regards,
Mike
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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] clk: Fix __GFP_FS allocation with irqs disabled
On 12/22/2014 10:38 AM, Stephen Boyd wrote:
> On 12/22/2014 03:45 AM, Krzysztof Kozlowski wrote:
>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index f4963b7d4e17..35079302a650 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
>> struct clk *parent)
>> {
>> int i;
>>
>> +if (clk->num_parents == 1) {
>> +if (IS_ERR_OR_NULL(clk->parent))
>> +clk->parent = __clk_lookup(clk->parent_names[0]);
>> +return 0;
>> +}
>> +
>> if (!clk->parents) {
>> clk->parents = kcalloc(clk->num_parents,
>> sizeof(struct clk *), GFP_KERNEL);
> This may be a worthwhile optimization, but I wonder why the clk_ops for
> this clock need to fetch the parent index at all? Which clock are we
> actually dealing with here?
>
The clk_set_rate() call should still be fixed, but we can probably do
this too.
---8<---
From: Stephen Boyd
Date: Mon, 22 Dec 2014 11:24:28 -0800
Subject: [PATCH] clk: Skip fetching index for single parent clocks
We don't need to fetch the parent index for clocks if they only
have one parent. Doing this also avoid an unnecessary allocation
for the parent cache.
Signed-off-by: Stephen Boyd
---
drivers/clk/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 675f37a7329f..e3a2d36124fd 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1420,7 +1420,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
unsigned long rate)
}
/* try finding the new parent index */
- if (parent) {
+ if (parent && clk->num_parents > 1) {
p_index = clk_fetch_parent_index(clk, parent);
if (p_index < 0) {
pr_debug("%s: clk %s can not be parent of clk %s\n",
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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] clk: Fix __GFP_FS allocation with irqs disabled
On 12/22/2014 03:45 AM, Krzysztof Kozlowski wrote:
> Allocation of memory for cached clock parents during s3c-sdhci probe was
> done with interrupts disabled which lead to lockdep warning:
>
> [2.254980] s3c-sdhci 1253.sdhci: clock source 2: mmc_busclk.2
> (5000 Hz)
> [2.560726] [ cut here ]
> [2.565341] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2744
> lockdep_trace_alloc+0xec/0x118()
> [2.574439] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
> [2.579821] Modules linked in:
> [2.583038] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
> 3.18.0-next-20141216-2-g4ff197fc1902-dirty #1318
> [2.593796] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [2.599892] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [2.607612] [] (show_stack) from []
> (dump_stack+0x70/0xbc)
> [2.614822] [] (dump_stack) from []
> (warn_slowpath_common+0x74/0xb0)
> [2.622885] [] (warn_slowpath_common) from []
> (warn_slowpath_fmt+0x30/0x40)
> [2.631569] [] (warn_slowpath_fmt) from []
> (lockdep_trace_alloc+0xec/0x118)
> [2.640246] [] (lockdep_trace_alloc) from []
> (__kmalloc+0x3c/0x1cc)
> [2.648240] [] (__kmalloc) from []
> (clk_fetch_parent_index+0xb8/0xd4)
> [2.656390] [] (clk_fetch_parent_index) from []
> (clk_calc_new_rates+0xe0/0x1fc)
> [2.665415] [] (clk_calc_new_rates) from []
> (clk_calc_new_rates+0x1b4/0x1fc)
> [2.674181] [] (clk_calc_new_rates) from []
> (clk_set_rate+0x50/0xc8)
> [2.682265] [] (clk_set_rate) from []
> (sdhci_cmu_set_clock+0x68/0x16c)
clk_set_rate() takes a mutex, so that looks to be the real problem here.
Don't call clk_set_rate() in a context where irqs are disabled.
> [2.690503] [] (sdhci_cmu_set_clock) from []
> (sdhci_do_set_ios+0xf0/0x64c)
> [2.699095] [] (sdhci_do_set_ios) from []
> (sdhci_set_ios+0x20/0x2c)
> [2.707080] [] (sdhci_set_ios) from []
> (mmc_power_up+0x118/0x1fc)
> [2.714889] [] (mmc_power_up) from []
> (mmc_start_host+0x44/0x6c)
> [2.722615] [] (mmc_start_host) from []
> (mmc_add_host+0x58/0x7c)
> [2.730341] [] (mmc_add_host) from []
> (sdhci_add_host+0x968/0xd94)
> [2.738240] [] (sdhci_add_host) from []
> (sdhci_s3c_probe+0x354/0x52c)
> [2.746406] [] (sdhci_s3c_probe) from []
> (platform_drv_probe+0x48/0xa4)
> [2.754733] [] (platform_drv_probe) from []
> (driver_probe_device+0x13c/0x37c)
> [2.763585] [] (driver_probe_device) from []
> (__driver_attach+0x94/0x98)
> [2.772003] [] (__driver_attach) from []
> (bus_for_each_dev+0x54/0x88)
> [2.780163] [] (bus_for_each_dev) from []
> (bus_add_driver+0xe4/0x200)
> [2.788322] [] (bus_add_driver) from []
> (driver_register+0x78/0xf4)
> [2.796308] [] (driver_register) from []
> (do_one_initcall+0xac/0x1f0)
> [2.804473] [] (do_one_initcall) from []
> (kernel_init_freeable+0x10c/0x1d8)
> [2.813153] [] (kernel_init_freeable) from []
> (kernel_init+0x28/0x108)
> [2.821398] [] (kernel_init) from []
> (ret_from_fork+0x14/0x2c)
> [2.828939] ---[ end trace 03cc00e539849d1f ]---
>
> The memory for cached parents was not allocated during __clk_init()
> because the clock has only one parent and commit 9ca1c5a4bf41 ("clk:
> cache parent clocks only for muxes") made allocation skipped in such case.
>
> Be consequent also in clk_fetch_parent_index(): don't cache parents if
> there is only one.
>
> Signed-off-by: Krzysztof Kozlowski
> Fixes: 9ca1c5a4bf41 ("clk: cache parent clocks only for muxes")
> Cc:
> ---
> drivers/clk/clk.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index f4963b7d4e17..35079302a650 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
> struct clk *parent)
> {
> int i;
>
> + if (clk->num_parents == 1) {
> + if (IS_ERR_OR_NULL(clk->parent))
> + clk->parent = __clk_lookup(clk->parent_names[0]);
> + return 0;
> + }
> +
> if (!clk->parents) {
> clk->parents = kcalloc(clk->num_parents,
> sizeof(struct clk *), GFP_KERNEL);
This may be a worthwhile optimization, but I wonder why the clk_ops for
this clock need to fetch the parent index at all? Which clock are we
actually dealing with here?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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] clk: Fix __GFP_FS allocation with irqs disabled
Allocation of memory for cached clock parents during s3c-sdhci probe was
done with interrupts disabled which lead to lockdep warning:
[2.254980] s3c-sdhci 1253.sdhci: clock source 2: mmc_busclk.2 (5000
Hz)
[2.560726] [ cut here ]
[2.565341] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2744
lockdep_trace_alloc+0xec/0x118()
[2.574439] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
[2.579821] Modules linked in:
[2.583038] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
3.18.0-next-20141216-2-g4ff197fc1902-dirty #1318
[2.593796] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[2.599892] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[2.607612] [] (show_stack) from []
(dump_stack+0x70/0xbc)
[2.614822] [] (dump_stack) from []
(warn_slowpath_common+0x74/0xb0)
[2.622885] [] (warn_slowpath_common) from []
(warn_slowpath_fmt+0x30/0x40)
[2.631569] [] (warn_slowpath_fmt) from []
(lockdep_trace_alloc+0xec/0x118)
[2.640246] [] (lockdep_trace_alloc) from []
(__kmalloc+0x3c/0x1cc)
[2.648240] [] (__kmalloc) from []
(clk_fetch_parent_index+0xb8/0xd4)
[2.656390] [] (clk_fetch_parent_index) from []
(clk_calc_new_rates+0xe0/0x1fc)
[2.665415] [] (clk_calc_new_rates) from []
(clk_calc_new_rates+0x1b4/0x1fc)
[2.674181] [] (clk_calc_new_rates) from []
(clk_set_rate+0x50/0xc8)
[2.682265] [] (clk_set_rate) from []
(sdhci_cmu_set_clock+0x68/0x16c)
[2.690503] [] (sdhci_cmu_set_clock) from []
(sdhci_do_set_ios+0xf0/0x64c)
[2.699095] [] (sdhci_do_set_ios) from []
(sdhci_set_ios+0x20/0x2c)
[2.707080] [] (sdhci_set_ios) from []
(mmc_power_up+0x118/0x1fc)
[2.714889] [] (mmc_power_up) from []
(mmc_start_host+0x44/0x6c)
[2.722615] [] (mmc_start_host) from []
(mmc_add_host+0x58/0x7c)
[2.730341] [] (mmc_add_host) from []
(sdhci_add_host+0x968/0xd94)
[2.738240] [] (sdhci_add_host) from []
(sdhci_s3c_probe+0x354/0x52c)
[2.746406] [] (sdhci_s3c_probe) from []
(platform_drv_probe+0x48/0xa4)
[2.754733] [] (platform_drv_probe) from []
(driver_probe_device+0x13c/0x37c)
[2.763585] [] (driver_probe_device) from []
(__driver_attach+0x94/0x98)
[2.772003] [] (__driver_attach) from []
(bus_for_each_dev+0x54/0x88)
[2.780163] [] (bus_for_each_dev) from []
(bus_add_driver+0xe4/0x200)
[2.788322] [] (bus_add_driver) from []
(driver_register+0x78/0xf4)
[2.796308] [] (driver_register) from []
(do_one_initcall+0xac/0x1f0)
[2.804473] [] (do_one_initcall) from []
(kernel_init_freeable+0x10c/0x1d8)
[2.813153] [] (kernel_init_freeable) from []
(kernel_init+0x28/0x108)
[2.821398] [] (kernel_init) from []
(ret_from_fork+0x14/0x2c)
[2.828939] ---[ end trace 03cc00e539849d1f ]---
The memory for cached parents was not allocated during __clk_init()
because the clock has only one parent and commit 9ca1c5a4bf41 ("clk:
cache parent clocks only for muxes") made allocation skipped in such case.
Be consequent also in clk_fetch_parent_index(): don't cache parents if
there is only one.
Signed-off-by: Krzysztof Kozlowski
Fixes: 9ca1c5a4bf41 ("clk: cache parent clocks only for muxes")
Cc:
---
drivers/clk/clk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f4963b7d4e17..35079302a650 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
struct clk *parent)
{
int i;
+ if (clk->num_parents == 1) {
+ if (IS_ERR_OR_NULL(clk->parent))
+ clk->parent = __clk_lookup(clk->parent_names[0]);
+ return 0;
+ }
+
if (!clk->parents) {
clk->parents = kcalloc(clk->num_parents,
sizeof(struct clk *), GFP_KERNEL);
--
1.9.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/
[PATCH] clk: Fix __GFP_FS allocation with irqs disabled
Allocation of memory for cached clock parents during s3c-sdhci probe was
done with interrupts disabled which lead to lockdep warning:
[2.254980] s3c-sdhci 1253.sdhci: clock source 2: mmc_busclk.2 (5000
Hz)
[2.560726] [ cut here ]
[2.565341] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2744
lockdep_trace_alloc+0xec/0x118()
[2.574439] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
[2.579821] Modules linked in:
[2.583038] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
3.18.0-next-20141216-2-g4ff197fc1902-dirty #1318
[2.593796] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[2.599892] [c0014c44] (unwind_backtrace) from [c0011bbc]
(show_stack+0x10/0x14)
[2.607612] [c0011bbc] (show_stack) from [c04953b8]
(dump_stack+0x70/0xbc)
[2.614822] [c04953b8] (dump_stack) from [c0023444]
(warn_slowpath_common+0x74/0xb0)
[2.622885] [c0023444] (warn_slowpath_common) from [c0023514]
(warn_slowpath_fmt+0x30/0x40)
[2.631569] [c0023514] (warn_slowpath_fmt) from [c0063644]
(lockdep_trace_alloc+0xec/0x118)
[2.640246] [c0063644] (lockdep_trace_alloc) from [c00df52c]
(__kmalloc+0x3c/0x1cc)
[2.648240] [c00df52c] (__kmalloc) from [c0394970]
(clk_fetch_parent_index+0xb8/0xd4)
[2.656390] [c0394970] (clk_fetch_parent_index) from [c0394a6c]
(clk_calc_new_rates+0xe0/0x1fc)
[2.665415] [c0394a6c] (clk_calc_new_rates) from [c0394b40]
(clk_calc_new_rates+0x1b4/0x1fc)
[2.674181] [c0394b40] (clk_calc_new_rates) from [c0395408]
(clk_set_rate+0x50/0xc8)
[2.682265] [c0395408] (clk_set_rate) from [c0377708]
(sdhci_cmu_set_clock+0x68/0x16c)
[2.690503] [c0377708] (sdhci_cmu_set_clock) from [c03735cc]
(sdhci_do_set_ios+0xf0/0x64c)
[2.699095] [c03735cc] (sdhci_do_set_ios) from [c0373b48]
(sdhci_set_ios+0x20/0x2c)
[2.707080] [c0373b48] (sdhci_set_ios) from [c035ddf0]
(mmc_power_up+0x118/0x1fc)
[2.714889] [c035ddf0] (mmc_power_up) from [c035ecd0]
(mmc_start_host+0x44/0x6c)
[2.722615] [c035ecd0] (mmc_start_host) from [c035fd60]
(mmc_add_host+0x58/0x7c)
[2.730341] [c035fd60] (mmc_add_host) from [c037454c]
(sdhci_add_host+0x968/0xd94)
[2.738240] [c037454c] (sdhci_add_host) from [c0377b60]
(sdhci_s3c_probe+0x354/0x52c)
[2.746406] [c0377b60] (sdhci_s3c_probe) from [c0283b58]
(platform_drv_probe+0x48/0xa4)
[2.754733] [c0283b58] (platform_drv_probe) from [c02824e8]
(driver_probe_device+0x13c/0x37c)
[2.763585] [c02824e8] (driver_probe_device) from [c02827bc]
(__driver_attach+0x94/0x98)
[2.772003] [c02827bc] (__driver_attach) from [c0280a60]
(bus_for_each_dev+0x54/0x88)
[2.780163] [c0280a60] (bus_for_each_dev) from [c0281b48]
(bus_add_driver+0xe4/0x200)
[2.788322] [c0281b48] (bus_add_driver) from [c0282dfc]
(driver_register+0x78/0xf4)
[2.796308] [c0282dfc] (driver_register) from [c00089b0]
(do_one_initcall+0xac/0x1f0)
[2.804473] [c00089b0] (do_one_initcall) from [c0673d94]
(kernel_init_freeable+0x10c/0x1d8)
[2.813153] [c0673d94] (kernel_init_freeable) from [c0490058]
(kernel_init+0x28/0x108)
[2.821398] [c0490058] (kernel_init) from [c000f268]
(ret_from_fork+0x14/0x2c)
[2.828939] ---[ end trace 03cc00e539849d1f ]---
The memory for cached parents was not allocated during __clk_init()
because the clock has only one parent and commit 9ca1c5a4bf41 (clk:
cache parent clocks only for muxes) made allocation skipped in such case.
Be consequent also in clk_fetch_parent_index(): don't cache parents if
there is only one.
Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
Fixes: 9ca1c5a4bf41 (clk: cache parent clocks only for muxes)
Cc: sta...@vger.kernel.org
---
drivers/clk/clk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f4963b7d4e17..35079302a650 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
struct clk *parent)
{
int i;
+ if (clk-num_parents == 1) {
+ if (IS_ERR_OR_NULL(clk-parent))
+ clk-parent = __clk_lookup(clk-parent_names[0]);
+ return 0;
+ }
+
if (!clk-parents) {
clk-parents = kcalloc(clk-num_parents,
sizeof(struct clk *), GFP_KERNEL);
--
1.9.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/
Re: [PATCH] clk: Fix __GFP_FS allocation with irqs disabled
On 12/22/2014 03:45 AM, Krzysztof Kozlowski wrote:
Allocation of memory for cached clock parents during s3c-sdhci probe was
done with interrupts disabled which lead to lockdep warning:
[2.254980] s3c-sdhci 1253.sdhci: clock source 2: mmc_busclk.2
(5000 Hz)
[2.560726] [ cut here ]
[2.565341] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2744
lockdep_trace_alloc+0xec/0x118()
[2.574439] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
[2.579821] Modules linked in:
[2.583038] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
3.18.0-next-20141216-2-g4ff197fc1902-dirty #1318
[2.593796] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[2.599892] [c0014c44] (unwind_backtrace) from [c0011bbc]
(show_stack+0x10/0x14)
[2.607612] [c0011bbc] (show_stack) from [c04953b8]
(dump_stack+0x70/0xbc)
[2.614822] [c04953b8] (dump_stack) from [c0023444]
(warn_slowpath_common+0x74/0xb0)
[2.622885] [c0023444] (warn_slowpath_common) from [c0023514]
(warn_slowpath_fmt+0x30/0x40)
[2.631569] [c0023514] (warn_slowpath_fmt) from [c0063644]
(lockdep_trace_alloc+0xec/0x118)
[2.640246] [c0063644] (lockdep_trace_alloc) from [c00df52c]
(__kmalloc+0x3c/0x1cc)
[2.648240] [c00df52c] (__kmalloc) from [c0394970]
(clk_fetch_parent_index+0xb8/0xd4)
[2.656390] [c0394970] (clk_fetch_parent_index) from [c0394a6c]
(clk_calc_new_rates+0xe0/0x1fc)
[2.665415] [c0394a6c] (clk_calc_new_rates) from [c0394b40]
(clk_calc_new_rates+0x1b4/0x1fc)
[2.674181] [c0394b40] (clk_calc_new_rates) from [c0395408]
(clk_set_rate+0x50/0xc8)
[2.682265] [c0395408] (clk_set_rate) from [c0377708]
(sdhci_cmu_set_clock+0x68/0x16c)
clk_set_rate() takes a mutex, so that looks to be the real problem here.
Don't call clk_set_rate() in a context where irqs are disabled.
[2.690503] [c0377708] (sdhci_cmu_set_clock) from [c03735cc]
(sdhci_do_set_ios+0xf0/0x64c)
[2.699095] [c03735cc] (sdhci_do_set_ios) from [c0373b48]
(sdhci_set_ios+0x20/0x2c)
[2.707080] [c0373b48] (sdhci_set_ios) from [c035ddf0]
(mmc_power_up+0x118/0x1fc)
[2.714889] [c035ddf0] (mmc_power_up) from [c035ecd0]
(mmc_start_host+0x44/0x6c)
[2.722615] [c035ecd0] (mmc_start_host) from [c035fd60]
(mmc_add_host+0x58/0x7c)
[2.730341] [c035fd60] (mmc_add_host) from [c037454c]
(sdhci_add_host+0x968/0xd94)
[2.738240] [c037454c] (sdhci_add_host) from [c0377b60]
(sdhci_s3c_probe+0x354/0x52c)
[2.746406] [c0377b60] (sdhci_s3c_probe) from [c0283b58]
(platform_drv_probe+0x48/0xa4)
[2.754733] [c0283b58] (platform_drv_probe) from [c02824e8]
(driver_probe_device+0x13c/0x37c)
[2.763585] [c02824e8] (driver_probe_device) from [c02827bc]
(__driver_attach+0x94/0x98)
[2.772003] [c02827bc] (__driver_attach) from [c0280a60]
(bus_for_each_dev+0x54/0x88)
[2.780163] [c0280a60] (bus_for_each_dev) from [c0281b48]
(bus_add_driver+0xe4/0x200)
[2.788322] [c0281b48] (bus_add_driver) from [c0282dfc]
(driver_register+0x78/0xf4)
[2.796308] [c0282dfc] (driver_register) from [c00089b0]
(do_one_initcall+0xac/0x1f0)
[2.804473] [c00089b0] (do_one_initcall) from [c0673d94]
(kernel_init_freeable+0x10c/0x1d8)
[2.813153] [c0673d94] (kernel_init_freeable) from [c0490058]
(kernel_init+0x28/0x108)
[2.821398] [c0490058] (kernel_init) from [c000f268]
(ret_from_fork+0x14/0x2c)
[2.828939] ---[ end trace 03cc00e539849d1f ]---
The memory for cached parents was not allocated during __clk_init()
because the clock has only one parent and commit 9ca1c5a4bf41 (clk:
cache parent clocks only for muxes) made allocation skipped in such case.
Be consequent also in clk_fetch_parent_index(): don't cache parents if
there is only one.
Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
Fixes: 9ca1c5a4bf41 (clk: cache parent clocks only for muxes)
Cc: sta...@vger.kernel.org
---
drivers/clk/clk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f4963b7d4e17..35079302a650 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
struct clk *parent)
{
int i;
+ if (clk-num_parents == 1) {
+ if (IS_ERR_OR_NULL(clk-parent))
+ clk-parent = __clk_lookup(clk-parent_names[0]);
+ return 0;
+ }
+
if (!clk-parents) {
clk-parents = kcalloc(clk-num_parents,
sizeof(struct clk *), GFP_KERNEL);
This may be a worthwhile optimization, but I wonder why the clk_ops for
this clock need to fetch the parent index at all? Which clock are we
actually dealing with here?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel
Re: [PATCH] clk: Fix __GFP_FS allocation with irqs disabled
On 12/22/2014 10:38 AM, Stephen Boyd wrote:
On 12/22/2014 03:45 AM, Krzysztof Kozlowski wrote:
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f4963b7d4e17..35079302a650 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1150,6 +1150,12 @@ static int clk_fetch_parent_index(struct clk *clk,
struct clk *parent)
{
int i;
+if (clk-num_parents == 1) {
+if (IS_ERR_OR_NULL(clk-parent))
+clk-parent = __clk_lookup(clk-parent_names[0]);
+return 0;
+}
+
if (!clk-parents) {
clk-parents = kcalloc(clk-num_parents,
sizeof(struct clk *), GFP_KERNEL);
This may be a worthwhile optimization, but I wonder why the clk_ops for
this clock need to fetch the parent index at all? Which clock are we
actually dealing with here?
The clk_set_rate() call should still be fixed, but we can probably do
this too.
---8---
From: Stephen Boyd sb...@codeaurora.org
Date: Mon, 22 Dec 2014 11:24:28 -0800
Subject: [PATCH] clk: Skip fetching index for single parent clocks
We don't need to fetch the parent index for clocks if they only
have one parent. Doing this also avoid an unnecessary allocation
for the parent cache.
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
drivers/clk/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 675f37a7329f..e3a2d36124fd 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1420,7 +1420,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
unsigned long rate)
}
/* try finding the new parent index */
- if (parent) {
+ if (parent clk-num_parents 1) {
p_index = clk_fetch_parent_index(clk, parent);
if (p_index 0) {
pr_debug(%s: clk %s can not be parent of clk %s\n,
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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/
