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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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