Re: [PATCH v8 3/4] drivers/perf: add DesignWare PCIe PMU driver

[Shuai Xue <xueshuai@linux.alibaba.com>]

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