Re: [PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

2015-02-05 Thread Alban Browaeys
Le jeudi 05 février 2015 à 11:33 -0800, Stephen Boyd a écrit :
> On 02/05/15 10:24, Alban Browaeys wrote:
> > Put is called more than once, thus hlist_del ends up on a poisoned
> > list.
> 
> Why is clk_put() called more than once on the same clk pointer? Where
> does this happe
> 

With only patch 1 from this set applied I get another oops: 


[7.346612] Unable to handle kernel paging request at virtual address 
00200200
[7.353686] pgd = ebbf4000
[7.355939] [00200200] *pgd=
[7.359444] Internal error: Oops: 805 [#1] PREEMPT SMP ARM
[7.364914] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 
fuse
[7.372201] CPU: 3 PID: 102 Comm: systemd-modules Not tainted 
3.19.0-rc7-next-20150204-00053-g17e4521 #93
[7.381764] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[7.387832] task: eb0a8f00 ti: ebbfe000 task.ti: ebbfe000
[7.393213] PC is at __clk_put+0x64/0xdc
[7.397111] LR is at clk_prepare_lock+0x20/0x100
[7.401712] pc : []lr : []psr: 200f0053
[7.401712] sp : ebbffbb8  ip : ebbffba0  fp : ebbffbcc
[7.413185] r10: ee0ff600  r9 : 0001  r8 : 
[7.413188] r7 : ebbffbf8  r6 :   r5 : ee5c7d80  r4 : ee0ff600
[7.413195] r3 : 00100100  r2 : 00200200  r1 : 080007ff  r0 : 0001
[7.413200] Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment 
user
[7.413204] Control: 10c5387d  Table: 6bbf404a  DAC: 0015
[7.413208] Process systemd-modules (pid: 102, stack limit = 0xebbfe218)
[7.413211] Stack: (0xebbffbb8 to 0xebc0)
[7.413215] fba0:   
 ee106400
[7.413222] fbc0: ebbffbdc ebbffbd0 c055222c c0556114 ebbffc6c ebbffbe0 
c0558604 c0552220
[7.413228] fbe0: ebbffbf8 c01cc560 ebbbc310 ee2b5200 ebbffc14 c01ced00 
ee5e0d3c 0001
[7.413234] fc00: 0181 ee2b5200 ebbffc34 ebbbc100  c08c5790 
ee2b5200 ebbbc300
[7.413239] fc20: 0001 0006 ebbffc5c ebbffc38 c01ced00 c01cb2d0 
ed834850 
[7.413245] fc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8  
bf06c0b4 0006
[7.413251] fc60: ebbffc8c ebbffc70 c044213c c055846c ed834850 c0b61248 
c0b61254 c0aa82b8
[7.413257] fc80: ebbffcc4 ebbffc90 c043ff34 c0442120 ed834850 bf06c0b4 
ed834884 ed834850
[7.413263] fca0: bf06c0b4 ed834884  bf0631f0 eb3cfc00 c0a4f40c 
ebbffce4 ebbffcc8
[7.413269] fcc0: c044020c c043fdc8   bf06c0b4 c0440194 
ebbffd0c ebbffce8
[7.413275] fce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5720 bf06c0b4 
eb329cc0 c0a87448
[7.413280] fd00: ebbffd1c ebbffd10 c043fa14 c043de88 ebbffd44 ebbffd20 
c043f460 c043f9f4
[7.413286] fd20: bf069280 ebbffd30 bf06c0b4  bf0631e8 bf06c388 
ebbffd5c ebbffd48
[7.413292] fd40: c0440bfc c043f370 0001  ebbffd6c ebbffd60 
c0442094 c0440b50
[7.413298] fd60: ebbffdbc ebbffd70 bf04ca08 c044203c  bf065090 
 
[7.413303] fd80:       
c0a53b20 bf06c208
[7.413309] fda0: c0a53b20 bf04c950  c0a53b20 ebbffe4c ebbffdc0 
c0008b28 bf04c95c
[7.413315] fdc0: 001f  ebbffdec ebbffdd8 c00504f4 c006dde0 
ebbfe000 
[7.413321] fde0: ee002140 00d0 c06ed168 000c c0a50600 0001 
ebbffe4c ebbffe08
[7.413327] fe00: c01541d8 c0153934 ebbfe008 ebbffe08 0001 ebbfe008 
ee002140 dc8cb100
[7.41] fe20: 0001 bf06c208 0001 eb3cfd00 eb3cf000 0001 
14c3101c eb3cf008
[7.413339] fe40: ebbffe74 ebbffe50 c06ed1a4 c00089ec ebbffe74 ebbffe60 
c014496c ebbfff48
[7.413345] fe60: 0001 bf06c208 ebbfff3c ebbffe78 c00af61c c06ed140 
bf06c214 7fff
[7.413350] fe80: c00ac6a8 ebbfff48 ebbffeb4 f0473db8 0780 0777 
f0473e84 bf06c214
[7.413356] fea0: bf06c378 b6dc99f8 bf06c250 c0a4f40c c00ad024 c0169924 
 
[7.413362] fec0: bf063194 0009   6e72656b 6c65 
 
[7.413367] fee0:       
 
[7.413373] ff00:    dc8cb100 ebbfff2c  
0006 b6dc99f8
[7.413379] ff20: 017b c000fb64 ebbfe000  ebbfffa4 ebbfff40 
c00afdec c00ada4c
[7.413385] ff40: c0180738  f0454000 002bb414 f070ec6c f066643f 
f066dd80 00020390
[7.413390] ff60: 00026450 bf06c1f0 0001  002f 0030 
001a 
[7.413396] ff80: 0008   b6dca7d4 00028948 e1c5e100 
 ebbfffa8
[7.413402] ffa0: c000f9c0 c00afd54 b6dca7d4 00028948 0006 b6dc99f8 
 b6dca31c
[7.413408] ffc0: b6dca7d4 00028948 e1c5e100 017b 0002 00015964 
00015f34 0002e640
[7.413414] ffe0: bedc0268 bedc0258 b6dc3c4b b6e6cd42 600d0070 0006 
6f7fd821 6f7fdc21
[7.413427] [] (__clk_put) from [] (clk_put+0x18/0x1c)
[7.413438] [] (clk_put) from [] 
(of_clk_set_defaults+0x1a4/0x2ec)
[

Re: [PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

2015-02-05 Thread Stephen Boyd
On 02/05/15 10:24, Alban Browaeys wrote:
> Put is called more than once, thus hlist_del ends up on a poisoned
> list.

Why is clk_put() called more than once on the same clk pointer? Where
does this happe

>
> Move hlist_del to the __clk_release handler managed by kref instead
> of calling it in _clk_put.
>
> Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")
> Signed-off-by: Alban Browaeys 
> ---
>  drivers/clk/clk.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8f33722..f1d4b33 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
>   struct clk *clk = container_of(, struct clk, core);
>   int i = core->num_parents;
>  
> + hlist_del(>child_node);
> + clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
> +
>   kfree(core->parents);
>   while (--i >= 0)
>   kfree_const(core->parent_names[i]);
> @@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
>  
>   clk_prepare_lock();
>  
> - hlist_del(>child_node);

Ugh we should call this clks_node or something so that we don't confuse
it with child_node in clk_core.

> - clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
>   owner = clk->core->owner;
>   kref_put(>core->ref, __clk_release);
>  


-- 
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 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

2015-02-05 Thread Alban Browaeys
Put is called more than once, thus hlist_del ends up on a poisoned
list.

Move hlist_del to the __clk_release handler managed by kref instead
of calling it in _clk_put.

Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")
Signed-off-by: Alban Browaeys 
---
 drivers/clk/clk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8f33722..f1d4b33 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
struct clk *clk = container_of(, struct clk, core);
int i = core->num_parents;
 
+   hlist_del(>child_node);
+   clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
+
kfree(core->parents);
while (--i >= 0)
kfree_const(core->parent_names[i]);
@@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
 
clk_prepare_lock();
 
-   hlist_del(>child_node);
-   clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
owner = clk->core->owner;
kref_put(>core->ref, __clk_release);
 
-- 
2.1.4

--
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/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

2015-02-05 Thread Alban Browaeys
Le jeudi 05 février 2015 à 11:33 -0800, Stephen Boyd a écrit :
 On 02/05/15 10:24, Alban Browaeys wrote:
  Put is called more than once, thus hlist_del ends up on a poisoned
  list.
 
 Why is clk_put() called more than once on the same clk pointer? Where
 does this happe
 

With only patch 1 from this set applied I get another oops: 


[7.346612] Unable to handle kernel paging request at virtual address 
00200200
[7.353686] pgd = ebbf4000
[7.355939] [00200200] *pgd=
[7.359444] Internal error: Oops: 805 [#1] PREEMPT SMP ARM
[7.364914] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 
fuse
[7.372201] CPU: 3 PID: 102 Comm: systemd-modules Not tainted 
3.19.0-rc7-next-20150204-00053-g17e4521 #93
[7.381764] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[7.387832] task: eb0a8f00 ti: ebbfe000 task.ti: ebbfe000
[7.393213] PC is at __clk_put+0x64/0xdc
[7.397111] LR is at clk_prepare_lock+0x20/0x100
[7.401712] pc : [c055616c]lr : [c0552bfc]psr: 200f0053
[7.401712] sp : ebbffbb8  ip : ebbffba0  fp : ebbffbcc
[7.413185] r10: ee0ff600  r9 : 0001  r8 : 
[7.413188] r7 : ebbffbf8  r6 :   r5 : ee5c7d80  r4 : ee0ff600
[7.413195] r3 : 00100100  r2 : 00200200  r1 : 080007ff  r0 : 0001
[7.413200] Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment 
user
[7.413204] Control: 10c5387d  Table: 6bbf404a  DAC: 0015
[7.413208] Process systemd-modules (pid: 102, stack limit = 0xebbfe218)
[7.413211] Stack: (0xebbffbb8 to 0xebc0)
[7.413215] fba0:   
 ee106400
[7.413222] fbc0: ebbffbdc ebbffbd0 c055222c c0556114 ebbffc6c ebbffbe0 
c0558604 c0552220
[7.413228] fbe0: ebbffbf8 c01cc560 ebbbc310 ee2b5200 ebbffc14 c01ced00 
ee5e0d3c 0001
[7.413234] fc00: 0181 ee2b5200 ebbffc34 ebbbc100  c08c5790 
ee2b5200 ebbbc300
[7.413239] fc20: 0001 0006 ebbffc5c ebbffc38 c01ced00 c01cb2d0 
ed834850 
[7.413245] fc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8  
bf06c0b4 0006
[7.413251] fc60: ebbffc8c ebbffc70 c044213c c055846c ed834850 c0b61248 
c0b61254 c0aa82b8
[7.413257] fc80: ebbffcc4 ebbffc90 c043ff34 c0442120 ed834850 bf06c0b4 
ed834884 ed834850
[7.413263] fca0: bf06c0b4 ed834884  bf0631f0 eb3cfc00 c0a4f40c 
ebbffce4 ebbffcc8
[7.413269] fcc0: c044020c c043fdc8   bf06c0b4 c0440194 
ebbffd0c ebbffce8
[7.413275] fce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5720 bf06c0b4 
eb329cc0 c0a87448
[7.413280] fd00: ebbffd1c ebbffd10 c043fa14 c043de88 ebbffd44 ebbffd20 
c043f460 c043f9f4
[7.413286] fd20: bf069280 ebbffd30 bf06c0b4  bf0631e8 bf06c388 
ebbffd5c ebbffd48
[7.413292] fd40: c0440bfc c043f370 0001  ebbffd6c ebbffd60 
c0442094 c0440b50
[7.413298] fd60: ebbffdbc ebbffd70 bf04ca08 c044203c  bf065090 
 
[7.413303] fd80:       
c0a53b20 bf06c208
[7.413309] fda0: c0a53b20 bf04c950  c0a53b20 ebbffe4c ebbffdc0 
c0008b28 bf04c95c
[7.413315] fdc0: 001f  ebbffdec ebbffdd8 c00504f4 c006dde0 
ebbfe000 
[7.413321] fde0: ee002140 00d0 c06ed168 000c c0a50600 0001 
ebbffe4c ebbffe08
[7.413327] fe00: c01541d8 c0153934 ebbfe008 ebbffe08 0001 ebbfe008 
ee002140 dc8cb100
[7.41] fe20: 0001 bf06c208 0001 eb3cfd00 eb3cf000 0001 
14c3101c eb3cf008
[7.413339] fe40: ebbffe74 ebbffe50 c06ed1a4 c00089ec ebbffe74 ebbffe60 
c014496c ebbfff48
[7.413345] fe60: 0001 bf06c208 ebbfff3c ebbffe78 c00af61c c06ed140 
bf06c214 7fff
[7.413350] fe80: c00ac6a8 ebbfff48 ebbffeb4 f0473db8 0780 0777 
f0473e84 bf06c214
[7.413356] fea0: bf06c378 b6dc99f8 bf06c250 c0a4f40c c00ad024 c0169924 
 
[7.413362] fec0: bf063194 0009   6e72656b 6c65 
 
[7.413367] fee0:       
 
[7.413373] ff00:    dc8cb100 ebbfff2c  
0006 b6dc99f8
[7.413379] ff20: 017b c000fb64 ebbfe000  ebbfffa4 ebbfff40 
c00afdec c00ada4c
[7.413385] ff40: c0180738  f0454000 002bb414 f070ec6c f066643f 
f066dd80 00020390
[7.413390] ff60: 00026450 bf06c1f0 0001  002f 0030 
001a 
[7.413396] ff80: 0008   b6dca7d4 00028948 e1c5e100 
 ebbfffa8
[7.413402] ffa0: c000f9c0 c00afd54 b6dca7d4 00028948 0006 b6dc99f8 
 b6dca31c
[7.413408] ffc0: b6dca7d4 00028948 e1c5e100 017b 0002 00015964 
00015f34 0002e640
[7.413414] ffe0: bedc0268 bedc0258 b6dc3c4b b6e6cd42 600d0070 0006 
6f7fd821 6f7fdc21
[7.413427] [c055616c] (__clk_put) from [c055222c] (clk_put+0x18/0x1c)
[7.413438] [c055222c] (clk_put) from [c0558604] 

[PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

2015-02-05 Thread Alban Browaeys
Put is called more than once, thus hlist_del ends up on a poisoned
list.

Move hlist_del to the __clk_release handler managed by kref instead
of calling it in _clk_put.

Fixes: 1c8e600440c7 (clk: Add rate constraints to clocks)
Signed-off-by: Alban Browaeys pra...@yahoo.com
---
 drivers/clk/clk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8f33722..f1d4b33 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
struct clk *clk = container_of(core, struct clk, core);
int i = core-num_parents;
 
+   hlist_del(clk-child_node);
+   clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
+
kfree(core-parents);
while (--i = 0)
kfree_const(core-parent_names[i]);
@@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
 
clk_prepare_lock();
 
-   hlist_del(clk-child_node);
-   clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
owner = clk-core-owner;
kref_put(clk-core-ref, __clk_release);
 
-- 
2.1.4

--
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/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.

2015-02-05 Thread Stephen Boyd
On 02/05/15 10:24, Alban Browaeys wrote:
 Put is called more than once, thus hlist_del ends up on a poisoned
 list.

Why is clk_put() called more than once on the same clk pointer? Where
does this happe


 Move hlist_del to the __clk_release handler managed by kref instead
 of calling it in _clk_put.

 Fixes: 1c8e600440c7 (clk: Add rate constraints to clocks)
 Signed-off-by: Alban Browaeys pra...@yahoo.com
 ---
  drivers/clk/clk.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index 8f33722..f1d4b33 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
   struct clk *clk = container_of(core, struct clk, core);
   int i = core-num_parents;
  
 + hlist_del(clk-child_node);
 + clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
 +
   kfree(core-parents);
   while (--i = 0)
   kfree_const(core-parent_names[i]);
 @@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
  
   clk_prepare_lock();
  
 - hlist_del(clk-child_node);

Ugh we should call this clks_node or something so that we don't confuse
it with child_node in clk_core.

 - clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
   owner = clk-core-owner;
   kref_put(clk-core-ref, __clk_release);
  


-- 
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/