Add module_exit_stub() for phylink module.
Changes since v1:
- Introduce a helper macro for module_exit() boilerplate
This series is the combination of following:
v1 with empty phylink_exit():
https://lore.kernel.org/all/[email protected]/
v1 of module_exit_stub():
https://lore.kernel.org/all/[email protected]/
Gan, Yi Fang (2):
driver.h: add helper macro for module_exit() boilerplate
net: phylink: Add module_exit_stub()
drivers/net/phy/phylink.c | 1 +
include/linux/device/driver.h | 14 ++++++++++++++
2 files changed, 15 insertions(+)
--
2.34.1
For the modules need a module_init() but don't need to do
anything special in module_exit() might need to have an empty
module_exit(). This patch add a new macro module_exit_stub() to
replace the empty module_exit(). The macro is useful to remove
the module_exit() boilerplate.
Cc: <[email protected]> # 6.1+
Suggested-by: Lobakin, Aleksander <[email protected]>
Signed-off-by: Gan, Yi Fang <[email protected]>
---
include/linux/device/driver.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 7738f458995f..7d322eef501e 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -288,4 +288,18 @@ static int __init __driver##_init(void) \
} \
device_initcall(__driver##_init);
+/**
+ * module_exit_stub() - Helper macro for drivers that have init but don't
+ * do anything in exit. This eliminates some boilerplate.
+ * Each module may only use this macro once, and calling it replaces
+ * module_exit().
+ *
+ * @__driver: driver name
+ */
+#define module_exit_stub(__driver) \
+static void __exit __driver##_exit(void) \
+{ \
+} \
+module_exit(__driver##_exit)
+
#endif /* _DEVICE_DRIVER_H_ */
--
2.34.1
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.
Add module_exit_stub() in phylink for the module to be unloadable.
Fixes: eca68a3c7d05 ("net: phylink: pass supported host PHY interface modes to phylib for SFP's PHYs")
Cc: <[email protected]> # 6.1+
Signed-off-by: Gan, Yi Fang <[email protected]>
---
drivers/net/phy/phylink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 25c19496a336..823c9b43cd92 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3725,6 +3725,7 @@ static int __init phylink_init(void)
}
module_init(phylink_init);
+module_exit_stub(phylink);
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("phylink models the MAC to optional PHY connection");
--
2.34.1
On Thu, Dec 21, 2023 at 04:51:07PM +0800, Gan, Yi Fang wrote:
> Add module_exit_stub() for phylink module.
>
> Changes since v1:
> - Introduce a helper macro for module_exit() boilerplate
>
> This series is the combination of following:
> v1 with empty phylink_exit():
> https://lore.kernel.org/all/[email protected]/
> v1 of module_exit_stub():
> https://lore.kernel.org/all/[email protected]/
As I said before, no, this isn't ok. Why just resubmit a patch when
it's already been rejected?
This patch series should NOT be accepted as-is, you know this!
Also, you are not following the documented and REQUIRED rules for Intel
developers to be submitting kernel patches, so on that reason alone
these need to be rejected.
Please work with the Intel internel developers to do this correctly if
you wish to submit this again in the future.
greg k-h
On Thu, Dec 21, 2023 at 04:51:09PM +0800, Gan, Yi Fang wrote:
> 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.
You are missing justification it is actually safe to remove
phylink. Maybe Russell King deliberately did not implement
module_exit() because it can explode in interesting ways if it was?
Andrew
Hi Andrew,
The function phylink_init() is introduced by others.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.6.y&id=eca68a3c7d05b38b4e728cead0c49718f2bc1d5a
The module_exit() is added by referring to
https://elixir.bootlin.com/linux/latest/source/kernel/module/main.c#L738.
Since the macro function is rejected, I will send a V3.
Let's see if Rusell King has any comment. Thanks.
Best Regards,
Gan Yi Fang
> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Thursday, December 21, 2023 5:23 PM
> To: Gan, Yi Fang <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>; Andrew Halaney
> <[email protected]>; Javier Martinez Canillas <[email protected]>; John
> Stultz <[email protected]>; Rafael J . Wysocki <[email protected]>; Jens Axboe
> <[email protected]>; 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]>; Lai, Peter Jun Ann
> <[email protected]>; Choong, Yong Liang
> <[email protected]>
> Subject: Re: [PATCH net v2 2/2] net: phylink: Add module_exit_stub()
>
> On Thu, Dec 21, 2023 at 04:51:09PM +0800, Gan, Yi Fang wrote:
> > 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.
>
> You are missing justification it is actually safe to remove phylink. Maybe Russell
> King deliberately did not implement
> module_exit() because it can explode in interesting ways if it was?
>
> Andrew
On Thu, Jan 04, 2024 at 09:45:47AM +0000, Gan, Yi Fang wrote:
> Hi Andrew,
>
> The function phylink_init() is introduced by others.
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.6.y&id=eca68a3c7d05b38b4e728cead0c49718f2bc1d5a
> The module_exit() is added by referring to
> https://elixir.bootlin.com/linux/latest/source/kernel/module/main.c#L738.
> Since the macro function is rejected, I will send a V3.
> Let's see if Rusell King has any comment. Thanks.
Please don't top post when using linux kernel mailing lists.
> > You are missing justification it is actually safe to remove phylink. Maybe Russell
> > King deliberately did not implement
> > module_exit() because it can explode in interesting ways if it was?
You still need to explain why it is safe to implement it.
Andrew