* [PATCH 1/3] drivers/perf: arm_pmu: Use devm_ allocators
2016-12-21 10:19 [PATCH 0/3] arm perf: Support DEBUG_TEST_DRIVER_REMOVE Alexander Stein
@ 2016-12-21 10:19 ` Alexander Stein
2016-12-21 10:19 ` [PATCH 2/3] drivers/perf: arm_pmu: add arm_pmu_device_remove Alexander Stein
2016-12-21 10:19 ` [PATCH 3/3] arm: perf: Add platform driver removal function Alexander Stein
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Stein @ 2016-12-21 10:19 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Will Deacon, Mark Rutland, Russell King
Cc: Alexander Stein, linux-kernel, linux-arm-kernel
This eliminates several calls to kfree.
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] drivers/perf: arm_pmu: add arm_pmu_device_remove
2016-12-21 10:19 [PATCH 0/3] arm perf: Support DEBUG_TEST_DRIVER_REMOVE Alexander Stein
2016-12-21 10:19 ` [PATCH 1/3] drivers/perf: arm_pmu: Use devm_ allocators Alexander Stein
@ 2016-12-21 10:19 ` Alexander Stein
2016-12-21 13:38 ` Peter Zijlstra
2016-12-21 10:19 ` [PATCH 3/3] arm: perf: Add platform driver removal function Alexander Stein
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Stein @ 2016-12-21 10:19 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Will Deacon, Mark Rutland, Russell King
Cc: Alexander Stein, linux-kernel, linux-arm-kernel
Add ARM PMU removal function. This will be required by perf event drivers
when option DEBUG_TEST_DRIVER_REMOVE is enabled.
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
drivers/perf/arm_pmu.c | 14 ++++++++++++++
include/linux/perf/arm_pmu.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index a9bbdbf..b7ddc4c 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -1022,6 +1022,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
armpmu_init(pmu);
pmu->plat_device = pdev;
+ platform_set_drvdata(pdev, pmu);
if (node && (of_id = of_match_node(of_table, pdev->dev.of_node))) {
init_fn = of_id->data;
@@ -1073,6 +1074,19 @@ int arm_pmu_device_probe(struct platform_device *pdev,
return ret;
}
+int arm_pmu_device_remove(struct platform_device *pdev)
+{
+ struct arm_pmu *pmu = platform_get_drvdata(pdev);
+
+ __oprofile_cpu_pmu = NULL;
+
+ perf_pmu_unregister(&pmu->pmu);
+
+ cpu_pmu_destroy(pmu);
+
+ return 0;
+}
+
static int arm_pmu_hp_init(void)
{
int ret;
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 8462da2..8106f27 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -159,6 +159,8 @@ struct pmu_probe_info {
int arm_pmu_device_probe(struct platform_device *pdev,
const struct of_device_id *of_table,
const struct pmu_probe_info *probe_table);
+int arm_pmu_device_remove(struct platform_device *pdev);
+
#define ARMV8_PMU_PDEV_NAME "armv8-pmu"
--
2.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] drivers/perf: arm_pmu: add arm_pmu_device_remove
2016-12-21 10:19 ` [PATCH 2/3] drivers/perf: arm_pmu: add arm_pmu_device_remove Alexander Stein
@ 2016-12-21 13:38 ` Peter Zijlstra
2016-12-21 14:45 ` Alexander Stein
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2016-12-21 13:38 UTC (permalink / raw)
To: Alexander Stein
Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
Will Deacon, Mark Rutland, Russell King, linux-kernel,
linux-arm-kernel
On Wed, Dec 21, 2016 at 11:19:34AM +0100, Alexander Stein wrote:
> Add ARM PMU removal function. This will be required by perf event drivers
> when option DEBUG_TEST_DRIVER_REMOVE is enabled.
>
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> ---
> drivers/perf/arm_pmu.c | 14 ++++++++++++++
> include/linux/perf/arm_pmu.h | 2 ++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index a9bbdbf..b7ddc4c 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -1022,6 +1022,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
> armpmu_init(pmu);
>
> pmu->plat_device = pdev;
> + platform_set_drvdata(pdev, pmu);
>
> if (node && (of_id = of_match_node(of_table, pdev->dev.of_node))) {
> init_fn = of_id->data;
> @@ -1073,6 +1074,19 @@ int arm_pmu_device_probe(struct platform_device *pdev,
> return ret;
> }
>
> +int arm_pmu_device_remove(struct platform_device *pdev)
> +{
> + struct arm_pmu *pmu = platform_get_drvdata(pdev);
> +
> + __oprofile_cpu_pmu = NULL;
> +
> + perf_pmu_unregister(&pmu->pmu);
> +
> + cpu_pmu_destroy(pmu);
> +
> + return 0;
> +}
So normally, if there are events that use this pmu, we hold a reference
on its module, which avoids removal from happening.
How is that guarantee made by DEBUG_TEST_DRIVER_REMOVE ? Or will it
simply kill everything even though there's active events for the PMU?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] drivers/perf: arm_pmu: add arm_pmu_device_remove
2016-12-21 13:38 ` Peter Zijlstra
@ 2016-12-21 14:45 ` Alexander Stein
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Stein @ 2016-12-21 14:45 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
Will Deacon, Mark Rutland, Russell King, linux-kernel,
linux-arm-kernel
On Wednesday 21 December 2016 14:38:54, Peter Zijlstra wrote:
> On Wed, Dec 21, 2016 at 11:19:34AM +0100, Alexander Stein wrote:
> > Add ARM PMU removal function. This will be required by perf event drivers
> > when option DEBUG_TEST_DRIVER_REMOVE is enabled.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > ---
> >
> > drivers/perf/arm_pmu.c | 14 ++++++++++++++
> > include/linux/perf/arm_pmu.h | 2 ++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> > index a9bbdbf..b7ddc4c 100644
> > --- a/drivers/perf/arm_pmu.c
> > +++ b/drivers/perf/arm_pmu.c
> > @@ -1022,6 +1022,7 @@ int arm_pmu_device_probe(struct platform_device
> > *pdev,>
> > armpmu_init(pmu);
> >
> > pmu->plat_device = pdev;
> >
> > + platform_set_drvdata(pdev, pmu);
> >
> > if (node && (of_id = of_match_node(of_table, pdev->dev.of_node))) {
> >
> > init_fn = of_id->data;
> >
> > @@ -1073,6 +1074,19 @@ int arm_pmu_device_probe(struct platform_device
> > *pdev,>
> > return ret;
> >
> > }
> >
> > +int arm_pmu_device_remove(struct platform_device *pdev)
> > +{
> > + struct arm_pmu *pmu = platform_get_drvdata(pdev);
> > +
> > + __oprofile_cpu_pmu = NULL;
> > +
> > + perf_pmu_unregister(&pmu->pmu);
> > +
> > + cpu_pmu_destroy(pmu);
> > +
> > + return 0;
> > +}
>
> So normally, if there are events that use this pmu, we hold a reference
> on its module, which avoids removal from happening.
>
> How is that guarantee made by DEBUG_TEST_DRIVER_REMOVE ? Or will it
> simply kill everything even though there's active events for the PMU?
AFAICS you won't be able to hold any reference until this test remove is done.
This feature is implemented in really_probe(). If the driver is successfully
probed it will be removed and probed again.
But reading that part of the code I stumbled over suppress_bind_attrs which
would prevent this procedure. After some grepping I found commit 80c6397c3
("clk: oxnas: make it explicitly non-modular").
Essentially setting
> .suppress_bind_attrs = true
in the platform_driver.
IMHO this seems far better than to add some remove functions only for testing
a non-removable driver. I'll come up with a 2nd series, patch 1/3 is still
valid.
Best regards,
Alexander
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] arm: perf: Add platform driver removal function
2016-12-21 10:19 [PATCH 0/3] arm perf: Support DEBUG_TEST_DRIVER_REMOVE Alexander Stein
2016-12-21 10:19 ` [PATCH 1/3] drivers/perf: arm_pmu: Use devm_ allocators Alexander Stein
2016-12-21 10:19 ` [PATCH 2/3] drivers/perf: arm_pmu: add arm_pmu_device_remove Alexander Stein
@ 2016-12-21 10:19 ` Alexander Stein
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Stein @ 2016-12-21 10:19 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Will Deacon, Mark Rutland, Russell King
Cc: Alexander Stein, linux-kernel, linux-arm-kernel
Despite the driver cannot be built as a module using the option
DEBUG_TEST_DRIVER_REMOVE requires the drivers to actually remove the
device.
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
arch/arm/kernel/perf_event_v7.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index b942349..3ad02a2 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -2026,12 +2026,18 @@ static int armv7_pmu_device_probe(struct platform_device *pdev)
armv7_pmu_probe_table);
}
+static int armv7_pmu_device_remove(struct platform_device *pdev)
+{
+ return arm_pmu_device_remove(pdev);
+}
+
static struct platform_driver armv7_pmu_driver = {
.driver = {
.name = "armv7-pmu",
.of_match_table = armv7_pmu_of_device_ids,
},
.probe = armv7_pmu_device_probe,
+ .remove = armv7_pmu_device_remove,
};
static int __init register_armv7_pmu_driver(void)
--
2.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread