* [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init @ 2017-11-06 13:34 Arnd Bergmann 2017-11-12 21:16 ` Thomas Gleixner 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2017-11-06 13:34 UTC (permalink / raw) To: Daniel Lezcano, Thomas Gleixner Cc: Arnd Bergmann, Benjamin Gaignard, linux-kernel The newly added function triggers a harmless Kbuild warning because of a missing annotation: WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() The function timer_of_exit() references the function __init timer_clk_exit(). This is often because timer_of_exit lacks a __init annotation or the annotation of timer_clk_exit is wrong. The function is only called from other __init functions, so it can safely be marked as __init as well. Fixes: f48729a999ee ("clocksource/drivers/timer-of: Add timer_of_exit function") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/clocksource/timer-of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c index 7c64a5c1bfc1..e301fdb1286e 100644 --- a/drivers/clocksource/timer-of.c +++ b/drivers/clocksource/timer-of.c @@ -177,7 +177,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to) return ret; } -void timer_of_exit(struct timer_of *to) +void __init timer_of_exit(struct timer_of *to) { if (to->flags & TIMER_OF_IRQ) timer_irq_exit(&to->of_irq); -- 2.9.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-06 13:34 [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init Arnd Bergmann @ 2017-11-12 21:16 ` Thomas Gleixner 2017-11-12 22:24 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Thomas Gleixner @ 2017-11-12 21:16 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Daniel Lezcano, Benjamin Gaignard, Sudeep Holla, LKML On Mon, 6 Nov 2017, Arnd Bergmann wrote: > The newly added function triggers a harmless Kbuild warning because > of a missing annotation: > > WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() > The function timer_of_exit() references > the function __init timer_clk_exit(). > This is often because timer_of_exit lacks a __init > annotation or the annotation of timer_clk_exit is wrong. > > The function is only called from other __init functions, so it > can safely be marked as __init as well. Hmm. I don't see any caller at all. From the intention of the patch I assume this isn't designed for using from init functions, so we rather have to remove the __init annotations from the called functions. Sudeep posted a patch which does that: https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com Though I rather would know whether this function is going to be used at all and what the intention of this patch was. Benjamin???? Thanks, tglx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-12 21:16 ` Thomas Gleixner @ 2017-11-12 22:24 ` Arnd Bergmann 2017-11-12 22:27 ` Thomas Gleixner 2017-11-13 10:11 ` Sudeep Holla 0 siblings, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2017-11-12 22:24 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Daniel Lezcano, Benjamin Gaignard, Sudeep Holla, LKML On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Mon, 6 Nov 2017, Arnd Bergmann wrote: >> The newly added function triggers a harmless Kbuild warning because >> of a missing annotation: >> >> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() >> The function timer_of_exit() references >> the function __init timer_clk_exit(). >> This is often because timer_of_exit lacks a __init >> annotation or the annotation of timer_clk_exit is wrong. >> >> The function is only called from other __init functions, so it >> can safely be marked as __init as well. > > Hmm. I don't see any caller at all. From the intention of the patch I > assume this isn't designed for using from init functions, so we rather have > to remove the __init annotations from the called functions. > > Sudeep posted a patch which does that: > > https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com > > Though I rather would know whether this function is going to be used at > all and what the intention of this patch was. > > Benjamin???? My interpretation was that timer drivers are still supposed to be unregistered at module unload time, but that you might use the new timer_of_exit() in the failure path of whatever function calls timer_of_init() successfully when something fails in the next step. Sudeep's interpretation also makes sense, I had not thought of that, but I now found the patch that adds a user in an init function: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html It seems I guessed right and Sudeep guessed wrong (both by pure chance I admit). Both patches solve the problem, Sudeep's version is a little more robust in case we ever add a caller in an __exit function (which I think is currently not allowed), while mine saves a little bit of memory and matches the current usage better. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-12 22:24 ` Arnd Bergmann @ 2017-11-12 22:27 ` Thomas Gleixner 2017-11-13 9:27 ` Benjamin Gaignard 2017-11-13 10:11 ` Sudeep Holla 1 sibling, 1 reply; 9+ messages in thread From: Thomas Gleixner @ 2017-11-12 22:27 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Daniel Lezcano, Benjamin Gaignard, Sudeep Holla, LKML On Sun, 12 Nov 2017, Arnd Bergmann wrote: > On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, 6 Nov 2017, Arnd Bergmann wrote: > >> The newly added function triggers a harmless Kbuild warning because > >> of a missing annotation: > >> > >> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() > >> The function timer_of_exit() references > >> the function __init timer_clk_exit(). > >> This is often because timer_of_exit lacks a __init > >> annotation or the annotation of timer_clk_exit is wrong. > >> > >> The function is only called from other __init functions, so it > >> can safely be marked as __init as well. > > > > Hmm. I don't see any caller at all. From the intention of the patch I > > assume this isn't designed for using from init functions, so we rather have > > to remove the __init annotations from the called functions. > > > > Sudeep posted a patch which does that: > > > > https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com > > > > Though I rather would know whether this function is going to be used at > > all and what the intention of this patch was. > > > > Benjamin???? > > My interpretation was that timer drivers are still supposed to be unregistered > at module unload time, but that you might use the new timer_of_exit() > in the failure path of whatever function calls timer_of_init() successfully > when something fails in the next step. > > Sudeep's interpretation also makes sense, I had not thought of that, but > I now found the patch that adds a user in an init function: > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html > > It seems I guessed right and Sudeep guessed wrong (both by pure chance > I admit). Both patches solve the problem, Sudeep's version is a little > more robust in case we ever add a caller in an __exit function (which I > think is currently not allowed), while mine saves a little bit of memory > and matches the current usage better. Right, but if the only use case is the cleanup in an error path, then the function name is a misnomer. Thanks, tglx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-12 22:27 ` Thomas Gleixner @ 2017-11-13 9:27 ` Benjamin Gaignard 0 siblings, 0 replies; 9+ messages in thread From: Benjamin Gaignard @ 2017-11-13 9:27 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Arnd Bergmann, Daniel Lezcano, Sudeep Holla, LKML 2017-11-12 23:27 GMT+01:00 Thomas Gleixner <tglx@linutronix.de>: > On Sun, 12 Nov 2017, Arnd Bergmann wrote: >> On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> > On Mon, 6 Nov 2017, Arnd Bergmann wrote: >> >> The newly added function triggers a harmless Kbuild warning because >> >> of a missing annotation: >> >> >> >> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() >> >> The function timer_of_exit() references >> >> the function __init timer_clk_exit(). >> >> This is often because timer_of_exit lacks a __init >> >> annotation or the annotation of timer_clk_exit is wrong. >> >> >> >> The function is only called from other __init functions, so it >> >> can safely be marked as __init as well. >> > >> > Hmm. I don't see any caller at all. From the intention of the patch I >> > assume this isn't designed for using from init functions, so we rather have >> > to remove the __init annotations from the called functions. >> > >> > Sudeep posted a patch which does that: >> > >> > https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com >> > >> > Though I rather would know whether this function is going to be used at >> > all and what the intention of this patch was. >> > >> > Benjamin???? >> >> My interpretation was that timer drivers are still supposed to be unregistered >> at module unload time, but that you might use the new timer_of_exit() >> in the failure path of whatever function calls timer_of_init() successfully >> when something fails in the next step. >> >> Sudeep's interpretation also makes sense, I had not thought of that, but >> I now found the patch that adds a user in an init function: >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html >> >> It seems I guessed right and Sudeep guessed wrong (both by pure chance >> I admit). Both patches solve the problem, Sudeep's version is a little >> more robust in case we ever add a caller in an __exit function (which I >> think is currently not allowed), while mine saves a little bit of memory >> and matches the current usage better. > > Right, but if the only use case is the cleanup in an error path, then the > function name is a misnomer. > > Thanks, > > tglx The function is suppose to be called when you need to undo what have been done in timer_of_init(). That could happen in error case or when removing the module. Until now this function isn't called yet because it was part on my series to update stm32 timer and onky this patch has been merged. Benjamin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-12 22:24 ` Arnd Bergmann 2017-11-12 22:27 ` Thomas Gleixner @ 2017-11-13 10:11 ` Sudeep Holla 2017-11-13 14:58 ` Daniel Lezcano 1 sibling, 1 reply; 9+ messages in thread From: Sudeep Holla @ 2017-11-13 10:11 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Thomas Gleixner, Daniel Lezcano, Benjamin Gaignard, LKML On Sun, Nov 12, 2017 at 11:24:56PM +0100, Arnd Bergmann wrote: > On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, 6 Nov 2017, Arnd Bergmann wrote: > >> The newly added function triggers a harmless Kbuild warning because > >> of a missing annotation: > >> > >> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() > >> The function timer_of_exit() references > >> the function __init timer_clk_exit(). > >> This is often because timer_of_exit lacks a __init > >> annotation or the annotation of timer_clk_exit is wrong. > >> > >> The function is only called from other __init functions, so it > >> can safely be marked as __init as well. > > > > Hmm. I don't see any caller at all. From the intention of the patch I > > assume this isn't designed for using from init functions, so we rather have > > to remove the __init annotations from the called functions. > > > > Sudeep posted a patch which does that: > > > > https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com > > > > Though I rather would know whether this function is going to be used at > > all and what the intention of this patch was. > > > > Benjamin???? > > My interpretation was that timer drivers are still supposed to be unregistered > at module unload time, but that you might use the new timer_of_exit() > in the failure path of whatever function calls timer_of_init() successfully > when something fails in the next step. > > Sudeep's interpretation also makes sense, I had not thought of that, but > I now found the patch that adds a user in an init function: > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html > > It seems I guessed right and Sudeep guessed wrong (both by pure chance > I admit). Ah OK, I just went by name that it will be called by some exit/remove function. > Both patches solve the problem, Sudeep's version is a little > more robust in case we ever add a caller in an __exit function (which I > think is currently not allowed), while mine saves a little bit of memory > and matches the current usage better. > Agreed, may be if we add users which is called from init functions, the warning should disappear. Also as tglx suggested, we could rename if it's just used from init function error/exit paths. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-13 10:11 ` Sudeep Holla @ 2017-11-13 14:58 ` Daniel Lezcano 2017-11-13 15:10 ` Thomas Gleixner 2017-11-13 15:10 ` Sudeep Holla 0 siblings, 2 replies; 9+ messages in thread From: Daniel Lezcano @ 2017-11-13 14:58 UTC (permalink / raw) To: Sudeep Holla, Arnd Bergmann; +Cc: Thomas Gleixner, Benjamin Gaignard, LKML On 13/11/2017 11:11, Sudeep Holla wrote: > On Sun, Nov 12, 2017 at 11:24:56PM +0100, Arnd Bergmann wrote: >> On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >>> On Mon, 6 Nov 2017, Arnd Bergmann wrote: >>>> The newly added function triggers a harmless Kbuild warning because >>>> of a missing annotation: >>>> >>>> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit() >>>> The function timer_of_exit() references >>>> the function __init timer_clk_exit(). >>>> This is often because timer_of_exit lacks a __init >>>> annotation or the annotation of timer_clk_exit is wrong. >>>> >>>> The function is only called from other __init functions, so it >>>> can safely be marked as __init as well. >>> >>> Hmm. I don't see any caller at all. From the intention of the patch I >>> assume this isn't designed for using from init functions, so we rather have >>> to remove the __init annotations from the called functions. >>> >>> Sudeep posted a patch which does that: >>> >>> https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com >>> >>> Though I rather would know whether this function is going to be used at >>> all and what the intention of this patch was. >>> >>> Benjamin???? >> >> My interpretation was that timer drivers are still supposed to be unregistered >> at module unload time, but that you might use the new timer_of_exit() >> in the failure path of whatever function calls timer_of_init() successfully >> when something fails in the next step. >> >> Sudeep's interpretation also makes sense, I had not thought of that, but >> I now found the patch that adds a user in an init function: >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html >> >> It seems I guessed right and Sudeep guessed wrong (both by pure chance >> I admit). > > Ah OK, I just went by name that it will be called by some exit/remove > function. > >> Both patches solve the problem, Sudeep's version is a little >> more robust in case we ever add a caller in an __exit function (which I >> think is currently not allowed), while mine saves a little bit of memory >> and matches the current usage better. >> > > Agreed, may be if we add users which is called from init functions, the > warning should disappear. Also as tglx suggested, we could rename if it's > just used from init function error/exit paths. The drivers are not compiled as module AFAICT, the function will be called in the init error path. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-13 14:58 ` Daniel Lezcano @ 2017-11-13 15:10 ` Thomas Gleixner 2017-11-13 15:10 ` Sudeep Holla 1 sibling, 0 replies; 9+ messages in thread From: Thomas Gleixner @ 2017-11-13 15:10 UTC (permalink / raw) To: Daniel Lezcano; +Cc: Sudeep Holla, Arnd Bergmann, Benjamin Gaignard, LKML On Mon, 13 Nov 2017, Daniel Lezcano wrote: > On 13/11/2017 11:11, Sudeep Holla wrote: > > Agreed, may be if we add users which is called from init functions, the > > warning should disappear. Also as tglx suggested, we could rename if it's > > just used from init function error/exit paths. > > The drivers are not compiled as module AFAICT, the function will be > called in the init error path. Ok, if the consensus is that this is used for common cleanup in __init error handling, then please can someone submit a new version of this which renames the function to something like timer_of_cleanup() and add the __init annotation to it? And while at it please add a kernel doc comment to that effect. Thanks, tglx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init 2017-11-13 14:58 ` Daniel Lezcano 2017-11-13 15:10 ` Thomas Gleixner @ 2017-11-13 15:10 ` Sudeep Holla 1 sibling, 0 replies; 9+ messages in thread From: Sudeep Holla @ 2017-11-13 15:10 UTC (permalink / raw) To: Daniel Lezcano Cc: Arnd Bergmann, Sudeep Holla, Thomas Gleixner, Benjamin Gaignard, LKML On 13/11/17 14:58, Daniel Lezcano wrote: > On 13/11/2017 11:11, Sudeep Holla wrote: [..] >> >> Agreed, may be if we add users which is called from init functions, the >> warning should disappear. Also as tglx suggested, we could rename if it's >> just used from init function error/exit paths. > > The drivers are not compiled as module AFAICT, the function will be > called in the init error path. > Understood, IMO better to rename the functions as cleanup or something similar. exit made it sound differently and since there were no users, I assumed it to be used in remove/exit functions. Sorry for the noise. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-13 15:10 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-06 13:34 [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init Arnd Bergmann 2017-11-12 21:16 ` Thomas Gleixner 2017-11-12 22:24 ` Arnd Bergmann 2017-11-12 22:27 ` Thomas Gleixner 2017-11-13 9:27 ` Benjamin Gaignard 2017-11-13 10:11 ` Sudeep Holla 2017-11-13 14:58 ` Daniel Lezcano 2017-11-13 15:10 ` Thomas Gleixner 2017-11-13 15:10 ` Sudeep Holla
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.