In delete_module(), if mod->init callback is defined but mod->exit callback
is not defined, it will assume the module cannot be removed and return
EBUSY. The module_exit() is missing from current phylink module drive
causing failure while unloading it.
This patch introduces phylink_exit() for phylink module removal.
Fixes: eca68a3c7d05 ("net: phylink: pass supported host PHY interface modes to phylib for SFP's PHYs")
Cc: <[email protected]> # 6.1+
Signed-off-by: Lai Peter Jun Ann <[email protected]>
Signed-off-by: Gan, Yi Fang <[email protected]>
---
v1 -> v2:
Introduce a macro function to reduce the boilerplate
v2 -> v3:
Remove the macro function as it is rejected and fix the
format issue suggested from v1
drivers/net/phy/phylink.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 25c19496a336..4a05cda74d42 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3726,5 +3726,11 @@ static int __init phylink_init(void)
module_init(phylink_init);
+static void __exit phylink_exit(void)
+{
+}
+
+module_exit(phylink_exit);
+
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("phylink models the MAC to optional PHY connection");
--
2.34.1
On Thu, Jan 04, 2024 at 06:12:55PM +0800, Gan, Yi Fang wrote:
65;7401;1c> In delete_module(), if mod->init callback is defined but mod->exit callback
> is not defined, it will assume the module cannot be removed and return
> EBUSY. The module_exit() is missing from current phylink module drive
> causing failure while unloading it.
This is still missing the explanation why this is safe.
Andrew
---
pw-bot: cr
> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Thursday, January 4, 2024 9:05 PM
> To: Gan, Yi Fang <[email protected]>
> Cc: Russell King <[email protected]>; Heiner Kallweit
> <[email protected]>; David S . Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> Abeni <[email protected]>; Marek Beh?n <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Looi, Hong Aun
> <[email protected]>; Voon, Weifeng <[email protected]>; Song,
> Yoong Siang <[email protected]>; Choong, Yong Liang
> <[email protected]>
> Subject: Re: [PATCH net v3 1/1] net: phylink: Add module_exit()
>
> On Thu, Jan 04, 2024 at 06:12:55PM +0800, Gan, Yi Fang wrote:
> 65;7401;1c> In delete_module(), if mod->init callback is defined but mod->exit
> callback
> > is not defined, it will assume the module cannot be removed and return
> > EBUSY. The module_exit() is missing from current phylink module drive
> > causing failure while unloading it.
>
> This is still missing the explanation why this is safe.
>
>
> Andrew
>
> ---
> pw-bot: cr
Hi Andrew,
Regarding the justification on why it is safe to remove phylink,
we had done some memory leak check when unloading the phylink module.
root@localhost:~# lsmod | grep "phylink"
phylink 73728 0
root@localhost:~# rmmod phylink
root@localhost:~# echo scan > /sys/kernel/debug/kmemleak
root@localhost:~# cat /sys/kernel/debug/kmemleak
root@localhost:~#
So far, we didn't observe any memory leaking happened when unloading
phylink module. Is it sufficient or do you have any other suggestions to check
on whether the module is safe to remove?
Best Regards,
Gan Yi Fang
On Thu, Jan 11, 2024 at 06:38:58AM +0000, Gan, Yi Fang wrote:
> Hi Andrew,
>
> Regarding the justification on why it is safe to remove phylink,
> we had done some memory leak check when unloading the phylink module.
>
> root@localhost:~# lsmod | grep "phylink"
> phylink 73728 0
> root@localhost:~# rmmod phylink
> root@localhost:~# echo scan > /sys/kernel/debug/kmemleak
> root@localhost:~# cat /sys/kernel/debug/kmemleak
> root@localhost:~#
>
> So far, we didn't observe any memory leaking happened when unloading
> phylink module. Is it sufficient or do you have any other suggestions to check
> on whether the module is safe to remove?
I have no idea why one would think that memory leaks are in some way
related to whether a module can be removed or not. To me at least they
are two separate issues.
I'll continue waiting for the justification...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> Hi Andrew,
>
> Regarding the justification on why it is safe to remove phylink,
> we had done some memory leak check when unloading the phylink module.
>
> root@localhost:~# lsmod | grep "phylink"
> phylink 73728 0
> root@localhost:~# rmmod phylink
> root@localhost:~# echo scan > /sys/kernel/debug/kmemleak
> root@localhost:~# cat /sys/kernel/debug/kmemleak
> root@localhost:~#
>
> So far, we didn't observe any memory leaking happened when unloading
> phylink module. Is it sufficient or do you have any other suggestions to check
> on whether the module is safe to remove?
In general, leaked memory is safe. Being leaked, nothing is using
it. If nothing is using it, how can it cause an opps, corrupt a file
system, etc.
What you need to do is review all users of phylink, and determine if
any of them retains a pointer to anything which phylink manages and
will not be freed or uninitialized when it is unloaded. Is all polling
of GPIOs cleanly stopped? Are interrupt handlers disabled and
removed. Are PCS and MAC drivers cleanly unloaded first? Are hwmon
entries cleanly removed, taking into account that user space might
have them open? All ethtool ioctl/netlink calls are out of the code
before it is removed, etc.
Andrew