Messages in this thread | | | Subject | Re: [PATCH drivers/perf: hisi:] drivers/perf: hisi: fix NULL pointer issue when uninstall hns3 pmu driver | From | Yicong Yang <> | Date | Tue, 10 Oct 2023 17:32:03 +0800 |
| |
Hi Jijie,
On 2023/10/9 18:50, Jijie Shao wrote: > From: Hao Chen <chenhao418@huawei.com> > > When uninstall hns3 pmu driver, it will call cpuhp_state_remove_instance() > and then callback function hns3_pmu_offline_cpu() is called, it may cause > NULL pointer call trace when other driver is installing or uninstalling > concurrently. >
More information about the calltrace you've met and how to reproduce this? I'm not sure why other drivers are involved.
> As John Garry's opinion, cpuhp_state_remove_instance() is used for shared > interrupt, and using cpuhp_state_remove_instance_nocalls() is fine for PCIe > or HNS3 pmu. >
I'm a bit confused here. We need to update the using CPU and migrate the perf context as well as the interrupt affinity in cpuhp::teardown() callback, so it make sense to not call this on driver detachment. But I cannot figure out why this is related to the shared interrupt, more details?
> So, replace cpuhp_state_remove_instance() with > cpuhp_state_remove_instance_nocalls() to fix this problem. > > Fixes: 66637ab137b4 ("drivers/perf: hisi: add driver for HNS3 PMU") > Signed-off-by: Hao Chen <chenhao418@huawei.com> > Signed-off-by: Jijie Shao <shaojijie@huawei.com> > --- > drivers/perf/hisilicon/hns3_pmu.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/perf/hisilicon/hns3_pmu.c b/drivers/perf/hisilicon/hns3_pmu.c > index e0457d84af6b..16869bf5bf4c 100644 > --- a/drivers/perf/hisilicon/hns3_pmu.c > +++ b/drivers/perf/hisilicon/hns3_pmu.c > @@ -1556,8 +1556,8 @@ static int hns3_pmu_init_pmu(struct pci_dev *pdev, struct hns3_pmu *hns3_pmu) > ret = perf_pmu_register(&hns3_pmu->pmu, hns3_pmu->pmu.name, -1); > if (ret) { > pci_err(pdev, "failed to register perf PMU, ret = %d.\n", ret); > - cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE, > - &hns3_pmu->node); > + cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE, > + &hns3_pmu->node); > } > > return ret; > @@ -1568,8 +1568,8 @@ static void hns3_pmu_uninit_pmu(struct pci_dev *pdev) > struct hns3_pmu *hns3_pmu = pci_get_drvdata(pdev); > > perf_pmu_unregister(&hns3_pmu->pmu); > - cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE, > - &hns3_pmu->node); > + cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE, > + &hns3_pmu->node); > } Thanks.
| |