linux-kernel - Re: Lowering the log level in watchdog_dev

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171011184603.GB8756@roeck-us.net>
Date:   Wed, 11 Oct 2017 11:46:03 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Radu Rendec <rrendec@...sta.com>
Cc:     Wim Van Sebroeck <wim@...ana.be>, linux-watchdog@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: Lowering the log level in watchdog_dev_register when err==-EBUSY

On Wed, Oct 11, 2017 at 06:46:31PM +0100, Radu Rendec wrote:
> Hello,
> 
> In a project I'm working on we have a valid use case where we activate
> both the i6300esb and softdog watchdogs. We always activate i6300esb
> first (which uses the "legacy" watchdog API) and then softdog. This
> gets us two "error" level messages (coming from watchdog_cdev_register)
> although softdog falls back to the "new" API and registers its char
> device just fine.
> 
> Since watchdog_cdev_register/watchdog_dev_register seem to be used only
> by watchdog_register_device and the latter always falls back to the
> "new" API, I'm thinking about lowering the log level of these messages
> when err is -EBUSY. Something along the lines of:
> 
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -928,11 +928,14 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
>                 watchdog_miscdev.parent = wdd->parent;
>                 err = misc_register(&watchdog_miscdev);
>                 if (err != 0) {
> -                       pr_err("%s: cannot register miscdev on minor=%d (err=%d).\n",
> -                               wdd->info->identity, WATCHDOG_MINOR, err);
> -                       if (err == -EBUSY)
> -                               pr_err("%s: a legacy watchdog module is probably present.\n",
> -                                       wdd->info->identity);
> +                       if (err == -EBUSY) {
> +                               pr_info("%s: cannot register miscdev on minor=%d (err=%d).\n",
> +                                               wdd->info->identity, WATCHDOG_MINOR, err);
> +                               pr_info("%s: a legacy watchdog module is probably present.\n",
> +                                               wdd->info->identity);
> +                       } else
> +                               pr_err("%s: cannot register miscdev on minor=%d (err=%d).\n",
> +                                       wdd->info->identity, WATCHDOG_MINOR, err);
>                         old_wd_data = NULL;
>                         kfree(wd_data);
>                         return err;
> 
> Does this look like a good approach? If not, what would you recommend?
> In any case, I want to upstream the change, so better ask first :)
> 

I would suggest to convert the offending driver to use the watchdog subsystem
(and along the line remove the restriction of only supporting a single
instance). You have the hardware, so that should be a straightforward change.

Thanks,
Guenter