On Sat, Mar 30, 2024 at 08:00:32AM +0100, Arnd Bergmann wrote:
> On Fri, Mar 29, 2024, at 11:02, Uwe Kleine-König wrote:
> > The einj_driver driver is registered using platform_driver_probe(). In
> > this case it cannot get unbound via sysfs and it's ok to put the remove
> > callback into an exit section. To prevent the modpost warning about
> > einj_driver referencing .exit.text, mark the driver struct with
> > __refdata and explain the situation in a comment.
> >
> > This is an improvement over commit a24118a8a687 ("ACPI: APEI: EINJ: mark
> > remove callback as non-__exit") which recently addressed the same issue,
> > but picked a less optimal variant.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
>
> Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
>
> I noticed another curiosity:
>
> > static struct platform_device *einj_dev;
> > -static struct platform_driver einj_driver = {
> > - .remove_new = einj_remove,
> > +/*
> > + * einj_remove() lives in .exit.text. For drivers registered via
> > + * platform_driver_probe() this is ok because they cannot get unbound at
> > + * runtime. So mark the driver struct with __refdata to prevent modpost
> > + * triggering a section mismatch warning.
> > + */
> > +static struct platform_driver einj_driver __refdata = {
> > + .remove_new = __exit_p(einj_remove),
> > .driver = {
> > .name = "acpi-einj",
> > },
>
> I was wondering why this doesn't cause an "unused function"
> warning for einj_remove(), given that __exit_p() turns the
> reference into NULL.
>
> As it turns out, the __exit annotation marks the function as
> "__attribute__((used))", so it still gets put in the object
> file but then dropped by the linker. The __used annotation
> seems to predate the introduction of "__attribute__((unused))",
> which would seem more appropriate here, which would allow
> more dead-code elimination.
>
> The patch below gets rid of the __used annotation completely,
> which in turn uncovers some interesting bugs with __exit
> functions in built-in code that are never called from
> anywhere, like
>
> drivers/video/fbdev/asiliantfb.c:627:20: error: 'asiliantfb_exit' defined but not used [-Werror=unused-function]
Do you plan to follow up with the respective fixes? If not I can add it
to my list of things to clean up.
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 58cef4c2e59a..d0e6354f3050 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -82,7 +82,7 @@
> #define __exitused __used
> #endif
>
> -#define __exit __section(".exit.text") __exitused __cold notrace
> +#define __exit __section(".exit.text") __cold notrace
include/linux/init.h:82:1: error: '__exitused' defined but not used [-Werror=unused-macro]
:-) Apart from that: nice find.
> /* Used for MEMORY_HOTPLUG */
> #define __meminit __section(".meminit.text") __cold notrace \
> @@ -394,7 +394,7 @@ void __init parse_early_options(char *cmdline);
> #ifdef MODULE
> #define __exit_p(x) x
> #else
> -#define __exit_p(x) NULL
> +#define __exit_p(x) (0 ? (x) : NULL)
I remember wondering about __exit_p not using this idiom, but I didn't
follow that thought.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
