linux-kernel - Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h

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: <a45aeb79-7918-42e9-1c47-3cc631057a59@roeck-us.net>
Date:   Sun, 23 Feb 2020 06:08:06 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Anson Huang <anson.huang@....com>
Cc:     "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h

On 2/22/20 4:16 PM, Anson Huang wrote:
> Hi, Guenter
> 
>> Subject: Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of
>> init.h
>>
>> On Fri, Feb 21, 2020 at 10:00:30AM +0800, Anson Huang wrote:
>>> There is nothing in use from init.h, remove it.
>>>
>>
>> NACK, sorry; this driver uses __init and __exit_p.
> 
> Ah, yes, just notice them. But I don't understand why the .probe callback needs
> __init and .remove callback needs __exit_p? Should they need to be removed?
>   

That is not a matter of "needs". __init causes the code to be removed after
initialization. This is ok and desirable if it is known that the hardware is
built-in and will only ever be probed once.

exit_p causes the code to be removed if it is built into the kernel. This is
desirable and makes sense if the device is known to never be removed.

Having said that, what _is_ unnecessary is the remove function. Registration
could use devm_watchdog_register_device(), and the watchdog subsystem should
prevent removal if the watchdog is running. Plus, the removal function is
buggy: It doesn' call clk_disable_unprepare() (but that could be handled
with devm_add_action_or_reset() in the probe function). In my opinion,
fixing all that would be more valuable than trying to drop an include file.

Thanks,
Guenter