Changes in v2;
* Instead of adding a remove function which is unused otherwise, mark the driver
as non-remoable
Using DEBUG_TEST_DRIVER_REMOVE every driver gets probed, removed and probed
again. This breaks drivers without removal function, e.g. arm perf
resulting in the following dump:
------------[ cut here ]------------
WARNING: CPU: 1 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x6c/0x7c
sysfs: cannot create duplicate filename '/devices/armv7_cortex_a7'
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.0+ #212
Hardware name: Freescale LS1021A
Backtrace:
[<8020c044>] (dump_backtrace) from [<8020c2f0>] (show_stack+0x18/0x1c)
r7:00000009 r6:60000013 r5:80c1776c r4:00000000
[<8020c2d8>] (show_stack) from [<8046ead8>] (dump_stack+0x94/0xa8)
[<8046ea44>] (dump_stack) from [<8021ecd0>] (__warn+0xec/0x104)
r7:00000009 r6:8092efc8 r5:00000000 r4:bf04bd80
[<8021ebe4>] (__warn) from [<8021ed28>] (warn_slowpath_fmt+0x40/0x48)
r9:80a32848 r8:00000000 r7:00000000 r6:bf0ab780 r5:8091d590 r4:bf0df000
[<8021ecec>] (warn_slowpath_fmt) from [<8036d53c>] (sysfs_warn_dup+0x6c/0x7c)
r3:bf0df000 r2:8092ef94
[<8036d4d0>] (sysfs_warn_dup) from [<8036d628>] (sysfs_create_dir_ns+0x8c/0x9c)
r6:bf0ab780 r5:bf20ee08 r4:ffffffef
[<8036d59c>] (sysfs_create_dir_ns) from [<80471860>] (kobject_add_internal+0xa8/0x34c)
r6:bf0aa198 r5:00000000 r4:bf20ee08
[<804717b8>] (kobject_add_internal) from [<80471b54>] (kobject_add+0x50/0x94)
r7:00000000 r6:00000000 r5:00000000 r4:bf20ee08
[<80471b08>] (kobject_add) from [<80524590>] (device_add+0xe4/0x590)
r3:00000000 r2:00000000
r6:bf20ee00 r5:00000000 r4:bf20ee08
[<805244ac>] (device_add) from [<802a38a8>] (pmu_dev_alloc+0x88/0xd8)
r10:00000091 r9:80a32848 r8:00000000 r7:80a32840 r6:80c0ed3c r5:00000000
r4:bf1fe800
[<802a3820>] (pmu_dev_alloc) from [<80a0cd20>] (perf_event_sysfs_init+0x5c/0xb4)
r7:80a32840 r6:00000000 r5:bf1fe800 r4:80c0ede0
[<80a0ccc4>] (perf_event_sysfs_init) from [<80201778>] (do_one_initcall+0x48/0x178)
r6:80c45000 r5:80a0ccc4 r4:00000006
[<80201730>] (do_one_initcall) from [<80a00ecc>] (kernel_init_freeable+0x168/0x20c)
r8:80a00610 r7:80a32840 r6:80c45000 r5:80c45000 r4:00000006
[<80a00d64>] (kernel_init_freeable) from [<806febcc>] (kernel_init+0x10/0x11c)
r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:806febbc
r4:00000000
[<806febbc>] (kernel_init) from [<80208388>] (ret_from_fork+0x14/0x2c)
r5:806febbc r4:00000000
---[ end trace 9d251d389382804f ]---
This patchset marks the driver as explicitly non-remoavle preventing this error.
This disables bin/unbind for this driver as well.
Alexander Stein (2):
drivers/perf: arm_pmu: Use devm_ allocators
arm: perf: Mark as non-removable
arch/arm/kernel/perf_event_v7.c | 1 +
drivers/perf/arm_pmu.c | 14 ++++----------
2 files changed, 5 insertions(+), 10 deletions(-)
--
2.10.2
This driver can only built into the kernel. So disallow driver bind/unbind
and also prevent a kernel error in case DEBUG_TEST_DRIVER_REMOVE is
enabled.
Signed-off-by: Alexander Stein <[email protected]>
---
arch/arm/kernel/perf_event_v7.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index b942349..795e373 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -2029,6 +2029,7 @@ static int armv7_pmu_device_probe(struct platform_device *pdev)
static struct platform_driver armv7_pmu_driver = {
.driver = {
.name = "armv7-pmu",
+ .suppress_bind_attrs = true,
.of_match_table = armv7_pmu_of_device_ids,
},
.probe = armv7_pmu_device_probe,
--
2.10.2
This eliminates several calls to kfree.
Signed-off-by: Alexander Stein <[email protected]>
---
drivers/perf/arm_pmu.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index b37b572..a9bbdbf 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -917,7 +917,8 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
bool using_spi = false;
struct platform_device *pdev = pmu->plat_device;
- irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
+ irqs = devm_kcalloc(&pdev->dev, pdev->num_resources, sizeof(*irqs),
+ GFP_KERNEL);
if (!irqs)
return -ENOMEM;
@@ -939,7 +940,6 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
pr_err("PPI/SPI IRQ type mismatch for %s!\n",
dn->name);
of_node_put(dn);
- kfree(irqs);
return -EINVAL;
}
@@ -988,10 +988,8 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
int ret;
ret = irq_get_percpu_devid_partition(irq, &pmu->supported_cpus);
- if (ret) {
- kfree(irqs);
+ if (ret)
return ret;
- }
} else {
/* Otherwise default to all CPUs */
cpumask_setall(&pmu->supported_cpus);
@@ -1001,8 +999,6 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
/* If we matched up the IRQ affinities, use them to route the SPIs */
if (using_spi && i == pdev->num_resources)
pmu->irq_affinity = irqs;
- else
- kfree(irqs);
return 0;
}
@@ -1017,7 +1013,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
struct arm_pmu *pmu;
int ret = -ENODEV;
- pmu = kzalloc(sizeof(struct arm_pmu), GFP_KERNEL);
+ pmu = devm_kzalloc(&pdev->dev, sizeof(struct arm_pmu), GFP_KERNEL);
if (!pmu) {
pr_info("failed to allocate PMU device!\n");
return -ENOMEM;
@@ -1074,8 +1070,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
out_free:
pr_info("%s: failed to register PMU devices!\n",
of_node_full_name(node));
- kfree(pmu->irq_affinity);
- kfree(pmu);
return ret;
}
--
2.10.2
On Wed, Dec 21, 2016 at 04:03:38PM +0100, Alexander Stein wrote:
> Changes in v2;
> * Instead of adding a remove function which is unused otherwise, mark the driver
> as non-remoable
>
> Using DEBUG_TEST_DRIVER_REMOVE every driver gets probed, removed and probed
> again. This breaks drivers without removal function, e.g. arm perf
> resulting in the following dump:
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x6c/0x7c
> sysfs: cannot create duplicate filename '/devices/armv7_cortex_a7'
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.0+ #212
> Hardware name: Freescale LS1021A
> Backtrace:
> [<8020c044>] (dump_backtrace) from [<8020c2f0>] (show_stack+0x18/0x1c)
> r7:00000009 r6:60000013 r5:80c1776c r4:00000000
> [<8020c2d8>] (show_stack) from [<8046ead8>] (dump_stack+0x94/0xa8)
> [<8046ea44>] (dump_stack) from [<8021ecd0>] (__warn+0xec/0x104)
> r7:00000009 r6:8092efc8 r5:00000000 r4:bf04bd80
> [<8021ebe4>] (__warn) from [<8021ed28>] (warn_slowpath_fmt+0x40/0x48)
> r9:80a32848 r8:00000000 r7:00000000 r6:bf0ab780 r5:8091d590 r4:bf0df000
> [<8021ecec>] (warn_slowpath_fmt) from [<8036d53c>] (sysfs_warn_dup+0x6c/0x7c)
> r3:bf0df000 r2:8092ef94
> [<8036d4d0>] (sysfs_warn_dup) from [<8036d628>] (sysfs_create_dir_ns+0x8c/0x9c)
> r6:bf0ab780 r5:bf20ee08 r4:ffffffef
> [<8036d59c>] (sysfs_create_dir_ns) from [<80471860>] (kobject_add_internal+0xa8/0x34c)
> r6:bf0aa198 r5:00000000 r4:bf20ee08
> [<804717b8>] (kobject_add_internal) from [<80471b54>] (kobject_add+0x50/0x94)
> r7:00000000 r6:00000000 r5:00000000 r4:bf20ee08
> [<80471b08>] (kobject_add) from [<80524590>] (device_add+0xe4/0x590)
> r3:00000000 r2:00000000
> r6:bf20ee00 r5:00000000 r4:bf20ee08
> [<805244ac>] (device_add) from [<802a38a8>] (pmu_dev_alloc+0x88/0xd8)
> r10:00000091 r9:80a32848 r8:00000000 r7:80a32840 r6:80c0ed3c r5:00000000
> r4:bf1fe800
> [<802a3820>] (pmu_dev_alloc) from [<80a0cd20>] (perf_event_sysfs_init+0x5c/0xb4)
> r7:80a32840 r6:00000000 r5:bf1fe800 r4:80c0ede0
> [<80a0ccc4>] (perf_event_sysfs_init) from [<80201778>] (do_one_initcall+0x48/0x178)
> r6:80c45000 r5:80a0ccc4 r4:00000006
> [<80201730>] (do_one_initcall) from [<80a00ecc>] (kernel_init_freeable+0x168/0x20c)
> r8:80a00610 r7:80a32840 r6:80c45000 r5:80c45000 r4:00000006
> [<80a00d64>] (kernel_init_freeable) from [<806febcc>] (kernel_init+0x10/0x11c)
> r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:806febbc
> r4:00000000
> [<806febbc>] (kernel_init) from [<80208388>] (ret_from_fork+0x14/0x2c)
> r5:806febbc r4:00000000
> ---[ end trace 9d251d389382804f ]---
Please consider putting some of the above in patch 2's description, so
the reason for the patch doesn't get lost. If you want to reduce the
commit message, you could consider cutting the registers from the
backtrace (which are only useful when doing in-depth debugging, not for
illustrating the callpath.)
I'd like to see acks on both of these before I take patch 2, but I'm not
expecting that to happen until the new year.
Thanks.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Hi,
On Wed, Dec 21, 2016 at 04:03:40PM +0100, Alexander Stein wrote:
> This driver can only built into the kernel. So disallow driver bind/unbind
> and also prevent a kernel error in case DEBUG_TEST_DRIVER_REMOVE is
> enabled.
>
> Signed-off-by: Alexander Stein <[email protected]>
> ---
> arch/arm/kernel/perf_event_v7.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
> index b942349..795e373 100644
> --- a/arch/arm/kernel/perf_event_v7.c
> +++ b/arch/arm/kernel/perf_event_v7.c
> @@ -2029,6 +2029,7 @@ static int armv7_pmu_device_probe(struct platform_device *pdev)
> static struct platform_driver armv7_pmu_driver = {
> .driver = {
> .name = "armv7-pmu",
> + .suppress_bind_attrs = true,
> .of_match_table = armv7_pmu_of_device_ids,
> },
While this patch looks correct, the other perf_event_* drivers (e.g. those
under arch/arm/) will need similar treatment.
More generally, updating each and every driver in this manner seems like a
scattergun approach that is tiresome and error prone.
IMO, it would be vastly better for a higher layer to enforce that we don't
attempt to unbind drivers where the driver does not have a remove callback, as
is the case here (and I suspect most over cases where DEBUG_TEST_DRIVER_REMOVE
is blowing up).
Is there any reason that can't be enforced at the bus layer, say?
Thanks,
Mark.