Re: [PATCH v2] watchdog: core: Make use of devm_register_reboot_notifier()

From: Andrey Smirnov
Date: Wed Oct 18 2017 - 11:50:31 EST


On Tue, Oct 17, 2017 at 3:08 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 17 Oct 2017 08:50:12 -0700 Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote:
>
>> On Sat, Apr 15, 2017 at 11:11 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>> > On 04/11/2017 09:06 AM, Andrey Smirnov wrote:
>> >>
>> >> Save a bit of cleanup code by leveraging newly added
>> >> devm_register_reboot_notifier().
>> >>
>> >> Cc: cphealy@xxxxxxxxx
>> >> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> >> Cc: Wim Van Sebroeck <wim@xxxxxxxxx>
>> >> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
>> >> Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
>> >> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
>> >
>> >
>> > Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>> >
>>
>> Andrew,
>>
>> This patch can't go in via watchdog tree because it depends on
>> devm_register_reboot_notifier() which is still present only in
>> linux-next, any change you can pull this into linux-next as well?
>
> That's because I've been sitting on
> kernel-reboot-add-devm_register_reboot_notifier.patch since March
> because it has no users.
>

Ah! I suspected that was the case.

> This patch adds a user. Have you identified other sites which
> can/should use devm_register_reboot_notifier()? I guess quite a lot,
> so it's a matter of alerting developers to the new interface. I
> wonder how. A checkpatch rule would do it, but that's new ground for
> checkpatch.
>

I can't say I looked for it in every nook and cranny, but I did look
at ~80% of the results that grepping for "register_reboot_notifier("
produces and it looked like lion's share of the code calling the
function didn't have a device to tie the registration lifespan to. For
dozen or so cases where there was a device to use, majority had the
call being done in the middle of probe/removal sequence, so it was
hard to tell if using devm_ version would be possible and if it would
give any significant benefits code reduction wise. However, there were
three places that looked like they had a pretty clear case for
conversion:

- drivers/input/misc/pm8941-pwrkey.c looked like a poster child for
the conversion, since the only thing it does in .remov is call
unregister_reboot_notifier()
- drivers/soc/lantiq/gphy.c and drivers/rtc/rtc-m41t80.c both looked
like they can be converted as well

Hope this is helpful.

Please, let me know if there's more action I should take about it.

Thanks,
Andrey Smirnov