| Message ID | 20221115073636.25412-1-yuancan@huawei.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | x86/platform/intel/iosf_mbi: Fix error handling in iosf_mbi_init() | expand |
On Tue, Nov 15, 2022 at 9:38 AM Yuan Can <yuancan@huawei.com> wrote: > > A problem about modprobe iosf_mbi failed is triggered with the following > log given: > > debugfs: Directory 'iosf_sb' with parent '/' already present! > > The reason is that iosf_mbi_init() returns pci_register_driver() > directly without checking its return value, if pci_register_driver() > failed, it returns without removing debugfs, resulting the debugfs of > iosf_sb can never be created later. > > iosf_mbi_init() > iosf_mbi_dbg_init() # create debugfs > pci_register_driver() > driver_register() > bus_add_driver() > priv = kzalloc(...) # OOM happened > # return without remove debugfs and destroy workqueue > > Fix by remove debugfs and iosf_mbi_pm_qos when pci_register_driver() removing > returns error. > static int __init iosf_mbi_init(void) > { > + int ret; > + > iosf_debugfs_init(); > > cpu_latency_qos_add_request(&iosf_mbi_pm_qos, PM_QOS_DEFAULT_VALUE); > > - return pci_register_driver(&iosf_mbi_pci_driver); > + ret = pci_register_driver(&iosf_mbi_pci_driver); > + if (ret) { > + cpu_latency_qos_remove_request(&iosf_mbi_pm_qos); > + iosf_debugfs_remove(); > + } > + > + return ret; > } Can we rewrite it as if (ret) goto err_remove; return 0; err_remove: ... return ret; ?
在 2022/11/15 16:26, Andy Shevchenko 写道: > On Tue, Nov 15, 2022 at 9:38 AM Yuan Can <yuancan@huawei.com> wrote: >> A problem about modprobe iosf_mbi failed is triggered with the following >> log given: >> >> debugfs: Directory 'iosf_sb' with parent '/' already present! >> >> The reason is that iosf_mbi_init() returns pci_register_driver() >> directly without checking its return value, if pci_register_driver() >> failed, it returns without removing debugfs, resulting the debugfs of >> iosf_sb can never be created later. >> >> iosf_mbi_init() >> iosf_mbi_dbg_init() # create debugfs >> pci_register_driver() >> driver_register() >> bus_add_driver() >> priv = kzalloc(...) # OOM happened >> # return without remove debugfs and destroy workqueue >> >> Fix by remove debugfs and iosf_mbi_pm_qos when pci_register_driver() > removing > >> returns error. >> static int __init iosf_mbi_init(void) >> { >> + int ret; >> + >> iosf_debugfs_init(); >> >> cpu_latency_qos_add_request(&iosf_mbi_pm_qos, PM_QOS_DEFAULT_VALUE); >> >> - return pci_register_driver(&iosf_mbi_pci_driver); >> + ret = pci_register_driver(&iosf_mbi_pci_driver); >> + if (ret) { >> + cpu_latency_qos_remove_request(&iosf_mbi_pm_qos); >> + iosf_debugfs_remove(); >> + } >> + >> + return ret; >> } > Can we rewrite it as > > if (ret) > goto err_remove; > > return 0; > > err_remove: > ... > return ret; > > ? Thanks for these suggestions! will be changed in the next version.
diff --git a/arch/x86/platform/intel/iosf_mbi.c b/arch/x86/platform/intel/iosf_mbi.c index fdd49d70b437..98632ac3db2f 100644 --- a/arch/x86/platform/intel/iosf_mbi.c +++ b/arch/x86/platform/intel/iosf_mbi.c @@ -545,11 +545,19 @@ static struct pci_driver iosf_mbi_pci_driver = { static int __init iosf_mbi_init(void) { + int ret; + iosf_debugfs_init(); cpu_latency_qos_add_request(&iosf_mbi_pm_qos, PM_QOS_DEFAULT_VALUE); - return pci_register_driver(&iosf_mbi_pci_driver); + ret = pci_register_driver(&iosf_mbi_pci_driver); + if (ret) { + cpu_latency_qos_remove_request(&iosf_mbi_pm_qos); + iosf_debugfs_remove(); + } + + return ret; } static void __exit iosf_mbi_exit(void)
A problem about modprobe iosf_mbi failed is triggered with the following log given: debugfs: Directory 'iosf_sb' with parent '/' already present! The reason is that iosf_mbi_init() returns pci_register_driver() directly without checking its return value, if pci_register_driver() failed, it returns without removing debugfs, resulting the debugfs of iosf_sb can never be created later. iosf_mbi_init() iosf_mbi_dbg_init() # create debugfs pci_register_driver() driver_register() bus_add_driver() priv = kzalloc(...) # OOM happened # return without remove debugfs and destroy workqueue Fix by remove debugfs and iosf_mbi_pm_qos when pci_register_driver() returns error. Fixes: 8dc12f933c9d ("x86/iosf: Add debugfs support") Fixes: e09db3d241f8 ("x86: baytrail/cherrytrail: Rework and move P-Unit PMIC bus semaphore code") Signed-off-by: Yuan Can <yuancan@huawei.com> --- arch/x86/platform/intel/iosf_mbi.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)