linux-kernel - RE: [PATCH] ARM: SAMSUNG: use spin_lock_irqsave() in clk_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-id: <1b3701cd8049$f20e16f0$d62a44d0$%kim@samsung.com>
Date:	Wed, 22 Aug 2012 18:38:54 +0900
From:	Kukjin Kim <kgene.kim@...sung.com>
To:	'Mandeep Singh Baines' <msb@...omium.org>,
	linux-kernel@...r.kernel.org
Cc:	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org,
	'Ben Dooks' <ben-linux@...ff.org>,
	'Russell King' <linux@....linux.org.uk>,
	'Minho Ban' <mhban@...sung.com>,
	'Jaecheol Lee' <jc.lee@...sung.com>,
	'Sunyoung Kang' <sy0816.kang@...sung.com>,
	'Olof Johansson' <olofj@...omium.org>
Subject: RE: [PATCH] ARM: SAMSUNG: use spin_lock_irqsave() in clk_set_parent

Mandeep Singh Baines wrote:
> 
> From 0cdf3aff, "ARM: SAMSUNG: use spin_lock_irqsave() in
> clk_{enable,disable}":
> 
>   The clk_enable()and clk_disable() can be used process and ISR either.
>   And actually it is used for real product and other platforms use it
>   now. So spin_lock_irqsave() should be used instead.
> 
> We need to make a similar change in clk_set_parent(). Otherwise,
> you can potentially get spinlock recursion:
> 
> BUG: spinlock recursion on CPU#0, kinteractive/68
>  lock: 807832a8, .magic: dead4ead, .owner: kinteractive/68, .owner_cpu: 0
> [<80015f54>] (unwind_backtrace+0x0/0x128) from [<804f2914>]
> (dump_stack+0x20/0x24)
> [<804f2914>] (dump_stack+0x20/0x24) from [<804f57b8>]
(spin_dump+0x80/0x94)
> [<804f57b8>] (spin_dump+0x80/0x94) from [<804f57f8>] (spin_bug+0x2c/0x30)
> [<804f57f8>] (spin_bug+0x2c/0x30) from [<80222730>]
> (do_raw_spin_lock+0x54/0x150)
> [<80222730>] (do_raw_spin_lock+0x54/0x150) from [<804f96ec>]
> (_raw_spin_lock_irqsave+0x20/0x28)
> [<804f96ec>] (_raw_spin_lock_irqsave+0x20/0x28) from [<80022ea4>]
> (clk_enable+0x3c/0x84)
> [<80022ea4>] (clk_enable+0x3c/0x84) from [<8038336c>]
> (s5p_mfc_clock_on+0x60/0x74)
> [<8038336c>] (s5p_mfc_clock_on+0x60/0x74) from [<8038645c>]
> (s5p_mfc_read_info+0x20/0x38)
> [<8038645c>] (s5p_mfc_read_info+0x20/0x38) from [<8037ca3c>]
> (s5p_mfc_handle_frame+0x2e4/0x4bc)
> [<8037ca3c>] (s5p_mfc_handle_frame+0x2e4/0x4bc) from [<8037d420>]
> (s5p_mfc_irq+0x1ec/0x6cc)
> [<8037d420>] (s5p_mfc_irq+0x1ec/0x6cc) from [<8007fc74>]
> (handle_irq_event_percpu+0x8c/0x244)
> [<8007fc74>] (handle_irq_event_percpu+0x8c/0x244) from [<8007fe78>]
> (handle_irq_event+0x4c/0x6c)
> [<8007fe78>] (handle_irq_event+0x4c/0x6c) from [<80082dd8>]
> (handle_fasteoi_irq+0xe4/0x150)
> [<80082dd8>] (handle_fasteoi_irq+0xe4/0x150) from [<8007f424>]
> (generic_handle_irq+0x3c/0x50)
> [<8007f424>] (generic_handle_irq+0x3c/0x50) from [<8000f7c4>]
> (handle_IRQ+0x88/0xc8)
> [<8000f7c4>] (handle_IRQ+0x88/0xc8) from [<80008564>]
> (gic_handle_irq+0x44/0x68)
> [<80008564>] (gic_handle_irq+0x44/0x68) from [<8000e400>]
> (__irq_svc+0x40/0x60)
> Exception stack(0xef3cbe68 to 0xef3cbeb0)
> [<8000e400>] (__irq_svc+0x40/0x60) from [<80022cfc>]
> (clk_set_parent+0x30/0x74)
> [<80022cfc>] (clk_set_parent+0x30/0x74) from [<803ac7f8>]
> (set_apll.isra.0+0x28/0xb0)
> [<803ac7f8>] (set_apll.isra.0+0x28/0xb0) from [<803ac8e4>]
> (exynos5250_set_frequency+0x64/0xb8)
> [<803ac8e4>] (exynos5250_set_frequency+0x64/0xb8) from [<803ac280>]
> (exynos_target+0x1b0/0x220)
> [<803ac280>] (exynos_target+0x1b0/0x220) from [<803a4a0c>]
> (__cpufreq_driver_target+0xb0/0xd4)
> [<803a4a0c>] (__cpufreq_driver_target+0xb0/0xd4) from [<803aab80>]
> (cpufreq_interactive_updown_task+0x214/0x264)
> [<803aab80>] (cpufreq_interactive_updown_task+0x214/0x264) from
> [<80047d04>] (kthread+0x9c/0xa8)
> [<80047d04>] (kthread+0x9c/0xa8) from [<8000fa48>]
> (kernel_thread_exit+0x0/0x8)
> 
> Signed-off-by: Mandeep Singh Baines <msb@...omium.org>
> Suggested-by: Sunil Mazhavanchery <sunilm@...sung.com>
> CC: linux-arm-kernel@...ts.infradead.org
> CC: linux-samsung-soc@...r.kernel.org
> CC: Ben Dooks <ben-linux@...ff.org>
> CC: Kukjin Kim <kgene.kim@...sung.com>
> CC: Russell King <linux@....linux.org.uk>
> CC: Minho Ban <mhban@...sung.com>
> CC: Jaecheol Lee <jc.lee@...sung.com>
> CC: Sunyoung Kang <sy0816.kang@...sung.com>
> CC: Kukjin Kim <kgene.kim@...sung.com>
> CC: Olof Johansson <olofj@...omium.org>
> ---
>  arch/arm/plat-samsung/clock.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-samsung/clock.c
> index 65c5eca..80eacca 100644
> --- a/arch/arm/plat-samsung/clock.c
> +++ b/arch/arm/plat-samsung/clock.c
> @@ -173,17 +173,18 @@ struct clk *clk_get_parent(struct clk *clk)
> 
>  int clk_set_parent(struct clk *clk, struct clk *parent)
>  {
> +	unsigned long flags;
>  	int ret = 0;
> 
>  	if (IS_ERR(clk))
>  		return -EINVAL;
> 
> -	spin_lock(&clocks_lock);
> +	spin_lock_irqsave(&clocks_lock, flags);
> 
>  	if (clk->ops && clk->ops->set_parent)
>  		ret = (clk->ops->set_parent)(clk, parent);
> 
> -	spin_unlock(&clocks_lock);
> +	spin_unlock_irqrestore(&clocks_lock, flags);
> 
>  	return ret;
>  }
> --
> 1.7.7.3

Yup, right. Applied.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@...sung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/