linux-kernel - Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+M3ks5PRhp=8-Qddq3ODtB5y8OikCGs_E=vC7K3ZYLe=o_O3g@mail.gmail.com>
Date:   Mon, 13 Nov 2017 10:27:51 +0100
From:   Benjamin Gaignard <benjamin.gaignard@...aro.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Sudeep Holla <sudeep.holla@....com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init

2017-11-12 23:27 GMT+01:00 Thomas Gleixner <tglx@...utronix.de>:
> On Sun, 12 Nov 2017, Arnd Bergmann wrote:
>> On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@...utronix.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