[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <dacd7394-c57a-44ee-a506-3ae0aa9870f9@linux.alibaba.com>
Date: Thu, 19 Oct 2023 20:02:34 +0800
From: Shuai Xue <xueshuai@...ux.alibaba.com>
To: Yicong Yang <yangyicong@...wei.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: yangyicong@...ilicon.com, chengyou@...ux.alibaba.com,
kaishen@...ux.alibaba.com, helgaas@...nel.org, will@...nel.org,
baolin.wang@...ux.alibaba.com, robin.murphy@....com,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-pci@...r.kernel.org, rdunlap@...radead.org,
mark.rutland@....com, zhuo.song@...ux.alibaba.com,
renyu.zj@...ux.alibaba.com
Subject: Re: [PATCH v8 3/4] drivers/perf: add DesignWare PCIe PMU driver
On 2023/10/19 16:05, Yicong Yang wrote:
> On 2023/10/18 11:33, Shuai Xue wrote:
>>
...
>>
>>>
>>>> + return PTR_ERR(dwc_pcie_pmu_dev);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void __exit dwc_pcie_pmu_exit(void)
>>>> +{
>>>> + platform_device_unregister(dwc_pcie_pmu_dev);
>>>> + platform_driver_unregister(&dwc_pcie_pmu_driver);
>>>> + cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state);
>>>> +
>>>> + if (dwc_pcie_pmu_notify)
>>>
>>> If you have something unusual like this a driver module_exit() it definitely
>>> deserves a comment on why. I'm surprised by this as I'd expect the notifier
>>> to be unregistered in the driver remove so not sure why this is here.
>>> I've lost track of earlier discussions so if this was addressed then all
>>> we need is a comment here for the next person to run into it!
>>
>> All replied above, I will unregistered the notifier by devm_add_action_or_reset().
>>
>> I am curious about that what the difference between unregistered in module_exit()
>> and remove()?
>>
>
> From my understanding, if you register it in probe() then should undo it in remove().
> Otherwise you should register it in module_init(). Just make them coupled to make
> sure cleanup the resources correctly.
>
> This driver is a bit different since device and driver are created in module_init()
> so will works fine in most cases, because the device/driver removal will happens the
> same time when unloading the module. However if manually unbind the driver and device
> without unloading the module, we'll miss to unregister the notifier in the currently
> implementation.
>
I see, thank you for your patient explanation.
Thank you.
Best Regards,
Shuai
Powered by blists - more mailing lists