2013-06-16 21:17:23

by Otavio Salvador

[permalink] [raw]
Subject: Fwd: mxs-lradc oops when unloading module

[ I am resending as it didn't make iio mailing list as the message
wasn't plain text ]

Hello,

When using mxs-lradc, as module, in a MX23 and unloading the driver,
it sometimes oops as:

[ 46.340000] Unable to handle kernel NULL pointer dereference at
virtual address 00000005
[ 46.340000] pgd = c7744000
[ 46.350000] [00000005] *pgd=46f4d831, *pte=00000000, *ppte=00000000
[ 46.360000] Internal error: Oops: 1 [#1] ARM
[ 46.360000] Modules linked in: mxs_lradc(C-) industrialio_triggered_buffer
[ 46.360000] CPU: 0 PID: 359 Comm: rmmod Tainted: G C
3.10.0-rc5+ #24
[ 46.360000] task: c6f6a200 ti: c680e000 task.ti: c680e000
[ 46.360000] PC is at module_put+0x18/0x80
[ 46.360000] LR is at iio_device_unregister_trigger_consumer+0x1c/0x28
[ 46.360000] pc : [<c0063334>] lr : [<c0373808>] psr: a0000013
[ 46.360000] sp : c680feb0 ip : 60000013 fp : 01324008
[ 46.360000] r10: 00000013 r9 : c680e000 r8 : 00000000
[ 46.360000] r7 : c6ee9418 r6 : c0373808 r5 : c6ee8800 r4 : c6ee9810
[ 46.360000] r3 : 00000001 r2 : c067a428 r1 : 000002e0 r0 : c6ee8800
[ 46.360000] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 46.360000] Control: 0005317f Table: 47744000 DAC: 00000015
[ 46.360000] Process rmmod (pid: 359, stack limit = 0xc680e1b8)
[ 46.360000] Stack: (0xc680feb0 to 0xc6810000)
[ 46.360000] fea0: c6ee9810
c6ee9400 c76cff80 c0373808
[ 46.360000] fec0: c6ee9410 c036f9bc c036f990 c6ee9418 c6ee9410
c02a9610 c6ee9434 c065fbe0
[ 46.360000] fee0: c77ccbe0 c024bc40 00000000 c6ee9400 00000000
c74c0644 c680e000 bf0049c4
[ 46.360000] ff00: bf004940 c74c0610 bf00589c c02ae058 c02ae044
c02ac8f0 bf00589c c74c0610
[ 46.360000] ff20: bf00589c c02ad0dc bf0058dc bf00589c c065fe60
c02ac730 bf0058dc 00000000
[ 46.360000] ff40: 00000800 c0064010 ffffffff 5f73786d 6461726c
00000063 c6f6a200 c680e000
[ 46.360000] ff60: c6f6a200 c000edcc 00000001 00000000 c680e000
bec16e48 01324008 c005c754
[ 46.360000] ff80: 01324088 00000800 01324088 01324088 00000800
01324088 00000081 c000ef08
[ 46.360000] ffa0: 00000000 c000ed40 01324088 00000800 013240b8
00000800 bec16c08 ffffeffc
[ 46.360000] ffc0: 01324088 00000800 01324088 00000081 00000001
bec16c38 bec16e48 01324008
[ 46.360000] ffe0: 4d819ef0 bec16bf4 4d808768 4d721e6c 20000010
013240b8 15401540 51110540
[ 46.360000] [<c0063334>] (module_put+0x18/0x80) from [<c0373808>]
(iio_device_unregister_trigger_consumer+0x1c/0x28)
[ 46.360000] [<c0373808>]
(iio_device_unregister_trigger_consumer+0x1c/0x28) from [<c036f9bc>]
(iio_dev_release+0x2c/0x6c)
[ 46.360000] [<c036f9bc>] (iio_dev_release+0x2c/0x6c) from
[<c02a9610>] (device_release+0x2c/0x90)
[ 46.360000] [<c02a9610>] (device_release+0x2c/0x90) from
[<c024bc40>] (kobject_release+0x48/0x7c)
[ 46.360000] [<c024bc40>] (kobject_release+0x48/0x7c) from
[<bf0049c4>] (mxs_lradc_remove+0x84/0x90 [mxs_lradc])
[ 46.360000] [<bf0049c4>] (mxs_lradc_remove+0x84/0x90 [mxs_lradc])
from [<c02ae058>] (platform_drv_remove+0x14/0x18)
[ 46.360000] [<c02ae058>] (platform_drv_remove+0x14/0x18) from
[<c02ac8f0>] (__device_release_driver+0x58/0xb4)
[ 46.360000] [<c02ac8f0>] (__device_release_driver+0x58/0xb4) from
[<c02ad0dc>] (driver_detach+0xb4/0xb8)
[ 46.360000] [<c02ad0dc>] (driver_detach+0xb4/0xb8) from
[<c02ac730>] (bus_remove_driver+0x7c/0xc0)
[ 46.360000] [<c02ac730>] (bus_remove_driver+0x7c/0xc0) from
[<c0064010>] (SyS_delete_module+0x150/0x26c)
[ 46.360000] [<c0064010>] (SyS_delete_module+0x150/0x26c) from
[<c000ed40>] (ret_fast_syscall+0x0/0x44)
[ 46.360000] Code: e1a0600e 08bd8070 e5953174 e59f2060 (e5931004)
[ 46.650000] ---[ end trace 7379015df3468e61 ]---

>From what I could grasp, it might be related to the IIO subsystem but
I couldn't find the culprit.

It seems "iio_device_unregister_trigger_consumer", as:

void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
{
/* Clean up an associated but not attached trigger reference */
if (indio_dev->trig)
iio_trigger_put(indio_dev->trig);
}

and , as:

static inline void iio_trigger_put(struct iio_trigger *trig)
{
module_put(trig->ops->owner);
put_device(&trig->dev);
}

so, somehow the trig->ops is NULL here.

Do someone has a clue?

--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://projetos.ossystems.com.br
Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750


2013-06-22 11:03:32

by Jonathan Cameron

[permalink] [raw]
Subject: Re: Fwd: mxs-lradc oops when unloading module

On 06/16/2013 10:09 PM, Otavio Salvador wrote:
> [ I am resending as it didn't make iio mailing list as the message
> wasn't plain text ]
>
> Hello,
>
> When using mxs-lradc, as module, in a MX23 and unloading the driver,
> it sometimes oops as:
>
> [ 46.340000] Unable to handle kernel NULL pointer dereference at
> virtual address 00000005
> [ 46.340000] pgd = c7744000
> [ 46.350000] [00000005] *pgd=46f4d831, *pte=00000000, *ppte=00000000
> [ 46.360000] Internal error: Oops: 1 [#1] ARM
> [ 46.360000] Modules linked in: mxs_lradc(C-) industrialio_triggered_buffer
> [ 46.360000] CPU: 0 PID: 359 Comm: rmmod Tainted: G C
> 3.10.0-rc5+ #24
> [ 46.360000] task: c6f6a200 ti: c680e000 task.ti: c680e000
> [ 46.360000] PC is at module_put+0x18/0x80
> [ 46.360000] LR is at iio_device_unregister_trigger_consumer+0x1c/0x28
> [ 46.360000] pc : [<c0063334>] lr : [<c0373808>] psr: a0000013
> [ 46.360000] sp : c680feb0 ip : 60000013 fp : 01324008
> [ 46.360000] r10: 00000013 r9 : c680e000 r8 : 00000000
> [ 46.360000] r7 : c6ee9418 r6 : c0373808 r5 : c6ee8800 r4 : c6ee9810
> [ 46.360000] r3 : 00000001 r2 : c067a428 r1 : 000002e0 r0 : c6ee8800
> [ 46.360000] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 46.360000] Control: 0005317f Table: 47744000 DAC: 00000015
> [ 46.360000] Process rmmod (pid: 359, stack limit = 0xc680e1b8)
> [ 46.360000] Stack: (0xc680feb0 to 0xc6810000)
> [ 46.360000] fea0: c6ee9810
> c6ee9400 c76cff80 c0373808
> [ 46.360000] fec0: c6ee9410 c036f9bc c036f990 c6ee9418 c6ee9410
> c02a9610 c6ee9434 c065fbe0
> [ 46.360000] fee0: c77ccbe0 c024bc40 00000000 c6ee9400 00000000
> c74c0644 c680e000 bf0049c4
> [ 46.360000] ff00: bf004940 c74c0610 bf00589c c02ae058 c02ae044
> c02ac8f0 bf00589c c74c0610
> [ 46.360000] ff20: bf00589c c02ad0dc bf0058dc bf00589c c065fe60
> c02ac730 bf0058dc 00000000
> [ 46.360000] ff40: 00000800 c0064010 ffffffff 5f73786d 6461726c
> 00000063 c6f6a200 c680e000
> [ 46.360000] ff60: c6f6a200 c000edcc 00000001 00000000 c680e000
> bec16e48 01324008 c005c754
> [ 46.360000] ff80: 01324088 00000800 01324088 01324088 00000800
> 01324088 00000081 c000ef08
> [ 46.360000] ffa0: 00000000 c000ed40 01324088 00000800 013240b8
> 00000800 bec16c08 ffffeffc
> [ 46.360000] ffc0: 01324088 00000800 01324088 00000081 00000001
> bec16c38 bec16e48 01324008
> [ 46.360000] ffe0: 4d819ef0 bec16bf4 4d808768 4d721e6c 20000010
> 013240b8 15401540 51110540
> [ 46.360000] [<c0063334>] (module_put+0x18/0x80) from [<c0373808>]
> (iio_device_unregister_trigger_consumer+0x1c/0x28)
> [ 46.360000] [<c0373808>]
> (iio_device_unregister_trigger_consumer+0x1c/0x28) from [<c036f9bc>]
> (iio_dev_release+0x2c/0x6c)
> [ 46.360000] [<c036f9bc>] (iio_dev_release+0x2c/0x6c) from
> [<c02a9610>] (device_release+0x2c/0x90)
> [ 46.360000] [<c02a9610>] (device_release+0x2c/0x90) from
> [<c024bc40>] (kobject_release+0x48/0x7c)
> [ 46.360000] [<c024bc40>] (kobject_release+0x48/0x7c) from
> [<bf0049c4>] (mxs_lradc_remove+0x84/0x90 [mxs_lradc])
> [ 46.360000] [<bf0049c4>] (mxs_lradc_remove+0x84/0x90 [mxs_lradc])
> from [<c02ae058>] (platform_drv_remove+0x14/0x18)
> [ 46.360000] [<c02ae058>] (platform_drv_remove+0x14/0x18) from
> [<c02ac8f0>] (__device_release_driver+0x58/0xb4)
> [ 46.360000] [<c02ac8f0>] (__device_release_driver+0x58/0xb4) from
> [<c02ad0dc>] (driver_detach+0xb4/0xb8)
> [ 46.360000] [<c02ad0dc>] (driver_detach+0xb4/0xb8) from
> [<c02ac730>] (bus_remove_driver+0x7c/0xc0)
> [ 46.360000] [<c02ac730>] (bus_remove_driver+0x7c/0xc0) from
> [<c0064010>] (SyS_delete_module+0x150/0x26c)
> [ 46.360000] [<c0064010>] (SyS_delete_module+0x150/0x26c) from
> [<c000ed40>] (ret_fast_syscall+0x0/0x44)
> [ 46.360000] Code: e1a0600e 08bd8070 e5953174 e59f2060 (e5931004)
> [ 46.650000] ---[ end trace 7379015df3468e61 ]---
>
> From what I could grasp, it might be related to the IIO subsystem but
> I couldn't find the culprit.
>
> It seems "iio_device_unregister_trigger_consumer", as:
>
> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> {
> /* Clean up an associated but not attached trigger reference */
> if (indio_dev->trig)
> iio_trigger_put(indio_dev->trig);
> }
>
> and , as:
>
> static inline void iio_trigger_put(struct iio_trigger *trig)
> {
> module_put(trig->ops->owner);
> put_device(&trig->dev);
> }
>
> so, somehow the trig->ops is NULL here.
>
> Do someone has a clue?
>
I think the issue is more core to iio trigger handling than that, you
have just been unlucky enough to hit a bug that has been there a while.

In iio_trigger_unregister we call
iio_device_unregister which in turn calls device_del (as intended) and
device_put (as not!). Then we get an additional device_put from
iio_trigger_free giving us one more than we should have an resulting
in a double attempt to free the device.

Could you try the following change and see if it solves the problem?
(the comment abov is 'interesting' and stupid given I've long ago
forgotten what it refers to, but it could be related to this issue...)

>From 238faf76acaa9214b0eb607742dd14b134469328 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <[email protected]>
Date: Sat, 22 Jun 2013 12:00:04 +0100
Subject: [PATCH] iio:trigger: device_unregister->device_del to avoid double
free

iio_trigger unregistration and freeing has been separated in this
code for some time, but it looks like the calls to the device
handling were not appropriately updated.

Signed-off-by: Jonathan Cameron <[email protected]>
---
drivers/iio/industrialio-trigger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 4d6c7d8..ea8a414 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -104,7 +104,7 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)

ida_simple_remove(&iio_trigger_ida, trig_info->id);
/* Possible issue in here */
- device_unregister(&trig_info->dev);
+ device_del(&trig_info->dev);
}
EXPORT_SYMBOL(iio_trigger_unregister);

--
1.8.3

2013-06-24 06:59:20

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: Fwd: mxs-lradc oops when unloading module

On 06/22/2013 01:03 PM, Jonathan Cameron wrote:
> On 06/16/2013 10:09 PM, Otavio Salvador wrote:
>> [ I am resending as it didn't make iio mailing list as the message
>> wasn't plain text ]
>>
>> Hello,
>>
>> When using mxs-lradc, as module, in a MX23 and unloading the driver,
>> it sometimes oops as:
>>
>> [ 46.340000] Unable to handle kernel NULL pointer dereference at
>> virtual address 00000005
>> [ 46.340000] pgd = c7744000
>> [ 46.350000] [00000005] *pgd=46f4d831, *pte=00000000, *ppte=00000000
>> [ 46.360000] Internal error: Oops: 1 [#1] ARM
>> [ 46.360000] Modules linked in: mxs_lradc(C-) industrialio_triggered_buffer
>> [ 46.360000] CPU: 0 PID: 359 Comm: rmmod Tainted: G C
>> 3.10.0-rc5+ #24
>> [ 46.360000] task: c6f6a200 ti: c680e000 task.ti: c680e000
>> [ 46.360000] PC is at module_put+0x18/0x80
>> [ 46.360000] LR is at iio_device_unregister_trigger_consumer+0x1c/0x28
>> [ 46.360000] pc : [<c0063334>] lr : [<c0373808>] psr: a0000013
>> [ 46.360000] sp : c680feb0 ip : 60000013 fp : 01324008
>> [ 46.360000] r10: 00000013 r9 : c680e000 r8 : 00000000
>> [ 46.360000] r7 : c6ee9418 r6 : c0373808 r5 : c6ee8800 r4 : c6ee9810
>> [ 46.360000] r3 : 00000001 r2 : c067a428 r1 : 000002e0 r0 : c6ee8800
>> [ 46.360000] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>> [ 46.360000] Control: 0005317f Table: 47744000 DAC: 00000015
>> [ 46.360000] Process rmmod (pid: 359, stack limit = 0xc680e1b8)
>> [ 46.360000] Stack: (0xc680feb0 to 0xc6810000)
>> [ 46.360000] fea0: c6ee9810
>> c6ee9400 c76cff80 c0373808
>> [ 46.360000] fec0: c6ee9410 c036f9bc c036f990 c6ee9418 c6ee9410
>> c02a9610 c6ee9434 c065fbe0
>> [ 46.360000] fee0: c77ccbe0 c024bc40 00000000 c6ee9400 00000000
>> c74c0644 c680e000 bf0049c4
>> [ 46.360000] ff00: bf004940 c74c0610 bf00589c c02ae058 c02ae044
>> c02ac8f0 bf00589c c74c0610
>> [ 46.360000] ff20: bf00589c c02ad0dc bf0058dc bf00589c c065fe60
>> c02ac730 bf0058dc 00000000
>> [ 46.360000] ff40: 00000800 c0064010 ffffffff 5f73786d 6461726c
>> 00000063 c6f6a200 c680e000
>> [ 46.360000] ff60: c6f6a200 c000edcc 00000001 00000000 c680e000
>> bec16e48 01324008 c005c754
>> [ 46.360000] ff80: 01324088 00000800 01324088 01324088 00000800
>> 01324088 00000081 c000ef08
>> [ 46.360000] ffa0: 00000000 c000ed40 01324088 00000800 013240b8
>> 00000800 bec16c08 ffffeffc
>> [ 46.360000] ffc0: 01324088 00000800 01324088 00000081 00000001
>> bec16c38 bec16e48 01324008
>> [ 46.360000] ffe0: 4d819ef0 bec16bf4 4d808768 4d721e6c 20000010
>> 013240b8 15401540 51110540
>> [ 46.360000] [<c0063334>] (module_put+0x18/0x80) from [<c0373808>]
>> (iio_device_unregister_trigger_consumer+0x1c/0x28)
>> [ 46.360000] [<c0373808>]
>> (iio_device_unregister_trigger_consumer+0x1c/0x28) from [<c036f9bc>]
>> (iio_dev_release+0x2c/0x6c)
>> [ 46.360000] [<c036f9bc>] (iio_dev_release+0x2c/0x6c) from
>> [<c02a9610>] (device_release+0x2c/0x90)
>> [ 46.360000] [<c02a9610>] (device_release+0x2c/0x90) from
>> [<c024bc40>] (kobject_release+0x48/0x7c)
>> [ 46.360000] [<c024bc40>] (kobject_release+0x48/0x7c) from
>> [<bf0049c4>] (mxs_lradc_remove+0x84/0x90 [mxs_lradc])
>> [ 46.360000] [<bf0049c4>] (mxs_lradc_remove+0x84/0x90 [mxs_lradc])
>> from [<c02ae058>] (platform_drv_remove+0x14/0x18)
>> [ 46.360000] [<c02ae058>] (platform_drv_remove+0x14/0x18) from
>> [<c02ac8f0>] (__device_release_driver+0x58/0xb4)
>> [ 46.360000] [<c02ac8f0>] (__device_release_driver+0x58/0xb4) from
>> [<c02ad0dc>] (driver_detach+0xb4/0xb8)
>> [ 46.360000] [<c02ad0dc>] (driver_detach+0xb4/0xb8) from
>> [<c02ac730>] (bus_remove_driver+0x7c/0xc0)
>> [ 46.360000] [<c02ac730>] (bus_remove_driver+0x7c/0xc0) from
>> [<c0064010>] (SyS_delete_module+0x150/0x26c)
>> [ 46.360000] [<c0064010>] (SyS_delete_module+0x150/0x26c) from
>> [<c000ed40>] (ret_fast_syscall+0x0/0x44)
>> [ 46.360000] Code: e1a0600e 08bd8070 e5953174 e59f2060 (e5931004)
>> [ 46.650000] ---[ end trace 7379015df3468e61 ]---
>>
>> From what I could grasp, it might be related to the IIO subsystem but
>> I couldn't find the culprit.
>>
>> It seems "iio_device_unregister_trigger_consumer", as:
>>
>> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
>> {
>> /* Clean up an associated but not attached trigger reference */
>> if (indio_dev->trig)
>> iio_trigger_put(indio_dev->trig);
>> }
>>
>> and , as:
>>
>> static inline void iio_trigger_put(struct iio_trigger *trig)
>> {
>> module_put(trig->ops->owner);
>> put_device(&trig->dev);
>> }
>>
>> so, somehow the trig->ops is NULL here.
>>
>> Do someone has a clue?
>>
> I think the issue is more core to iio trigger handling than that, you
> have just been unlucky enough to hit a bug that has been there a while.
>
> In iio_trigger_unregister we call
> iio_device_unregister which in turn calls device_del (as intended) and
> device_put (as not!). Then we get an additional device_put from
> iio_trigger_free giving us one more than we should have an resulting
> in a double attempt to free the device.
>
> Could you try the following change and see if it solves the problem?
> (the comment abov is 'interesting' and stupid given I've long ago
> forgotten what it refers to, but it could be related to this issue...)

Regardless of whether it fixes the problem or not the patch should be
applied, since it is definitely correct.

Reviewed-by: Lars-Peter Clausen <[email protected]>

>
> From 238faf76acaa9214b0eb607742dd14b134469328 Mon Sep 17 00:00:00 2001
> From: Jonathan Cameron <[email protected]>
> Date: Sat, 22 Jun 2013 12:00:04 +0100
> Subject: [PATCH] iio:trigger: device_unregister->device_del to avoid double
> free
>
> iio_trigger unregistration and freeing has been separated in this
> code for some time, but it looks like the calls to the device
> handling were not appropriately updated.
>
> Signed-off-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/industrialio-trigger.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 4d6c7d8..ea8a414 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -104,7 +104,7 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
>
> ida_simple_remove(&iio_trigger_ida, trig_info->id);
> /* Possible issue in here */
> - device_unregister(&trig_info->dev);
> + device_del(&trig_info->dev);
> }
> EXPORT_SYMBOL(iio_trigger_unregister);
>

2013-06-24 12:30:26

by Otavio Salvador

[permalink] [raw]
Subject: Re: Fwd: mxs-lradc oops when unloading module

On Sat, Jun 22, 2013 at 8:03 AM, Jonathan Cameron <[email protected]> wrote:
>> From what I could grasp, it might be related to the IIO subsystem but
>> I couldn't find the culprit.
>>
>> It seems "iio_device_unregister_trigger_consumer", as:
>>
>> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
>> {
>> /* Clean up an associated but not attached trigger reference */
>> if (indio_dev->trig)
>> iio_trigger_put(indio_dev->trig);
>> }
>>
>> and , as:
>>
>> static inline void iio_trigger_put(struct iio_trigger *trig)
>> {
>> module_put(trig->ops->owner);
>> put_device(&trig->dev);
>> }
>>
>> so, somehow the trig->ops is NULL here.
>>
>> Do someone has a clue?
>>
> I think the issue is more core to iio trigger handling than that, you
> have just been unlucky enough to hit a bug that has been there a while.
>
> In iio_trigger_unregister we call
> iio_device_unregister which in turn calls device_del (as intended) and
> device_put (as not!). Then we get an additional device_put from
> iio_trigger_free giving us one more than we should have an resulting
> in a double attempt to free the device.
>
> Could you try the following change and see if it solves the problem?
> (the comment abov is 'interesting' and stupid given I've long ago
> forgotten what it refers to, but it could be related to this issue...)
>
> From 238faf76acaa9214b0eb607742dd14b134469328 Mon Sep 17 00:00:00 2001
> From: Jonathan Cameron <[email protected]>
> Date: Sat, 22 Jun 2013 12:00:04 +0100
> Subject: [PATCH] iio:trigger: device_unregister->device_del to avoid double
> free
>
> iio_trigger unregistration and freeing has been separated in this
> code for some time, but it looks like the calls to the device
> handling were not appropriately updated.
>
> Signed-off-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/industrialio-trigger.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 4d6c7d8..ea8a414 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -104,7 +104,7 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
>
> ida_simple_remove(&iio_trigger_ida, trig_info->id);
> /* Possible issue in here */
> - device_unregister(&trig_info->dev);
> + device_del(&trig_info->dev);
> }
> EXPORT_SYMBOL(iio_trigger_unregister);

Tested-by: Otavio Salvador <[email protected]>

It does fix the Oops for me.

--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://projetos.ossystems.com.br
Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750

2013-06-29 12:27:40

by Jonathan Cameron

[permalink] [raw]
Subject: Re: Fwd: mxs-lradc oops when unloading module

On 06/24/2013 01:30 PM, Otavio Salvador wrote:
> On Sat, Jun 22, 2013 at 8:03 AM, Jonathan Cameron <[email protected]> wrote:
>>> From what I could grasp, it might be related to the IIO subsystem but
>>> I couldn't find the culprit.
>>>
>>> It seems "iio_device_unregister_trigger_consumer", as:
>>>
>>> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
>>> {
>>> /* Clean up an associated but not attached trigger reference */
>>> if (indio_dev->trig)
>>> iio_trigger_put(indio_dev->trig);
>>> }
>>>
>>> and , as:
>>>
>>> static inline void iio_trigger_put(struct iio_trigger *trig)
>>> {
>>> module_put(trig->ops->owner);
>>> put_device(&trig->dev);
>>> }
>>>
>>> so, somehow the trig->ops is NULL here.
>>>
>>> Do someone has a clue?
>>>
>> I think the issue is more core to iio trigger handling than that, you
>> have just been unlucky enough to hit a bug that has been there a while.
>>
>> In iio_trigger_unregister we call
>> iio_device_unregister which in turn calls device_del (as intended) and
>> device_put (as not!). Then we get an additional device_put from
>> iio_trigger_free giving us one more than we should have an resulting
>> in a double attempt to free the device.
>>
>> Could you try the following change and see if it solves the problem?
>> (the comment abov is 'interesting' and stupid given I've long ago
>> forgotten what it refers to, but it could be related to this issue...)
>>
>> From 238faf76acaa9214b0eb607742dd14b134469328 Mon Sep 17 00:00:00 2001
>> From: Jonathan Cameron <[email protected]>
>> Date: Sat, 22 Jun 2013 12:00:04 +0100
>> Subject: [PATCH] iio:trigger: device_unregister->device_del to avoid double
>> free
>>
>> iio_trigger unregistration and freeing has been separated in this
>> code for some time, but it looks like the calls to the device
>> handling were not appropriately updated.
>>
>> Signed-off-by: Jonathan Cameron <[email protected]>
>> ---
>> drivers/iio/industrialio-trigger.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
>> index 4d6c7d8..ea8a414 100644
>> --- a/drivers/iio/industrialio-trigger.c
>> +++ b/drivers/iio/industrialio-trigger.c
>> @@ -104,7 +104,7 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
>>
>> ida_simple_remove(&iio_trigger_ida, trig_info->id);
>> /* Possible issue in here */
>> - device_unregister(&trig_info->dev);
>> + device_del(&trig_info->dev);
>> }
>> EXPORT_SYMBOL(iio_trigger_unregister);
>
> Tested-by: Otavio Salvador <[email protected]>
>
> It does fix the Oops for me.
applied to the fixes-togreg branch of iio.git
Given timing these will probably go in after the merge window closes in a couple of
weeks.

Thanks,
>
> --
> Otavio Salvador O.S. Systems
> http://www.ossystems.com.br http://projetos.ossystems.com.br
> Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>