[PATCH] softdog.c (was: Kernel panic after rmmod softdog (2.6.8.1))

[PATCH] softdog.c (was: Kernel panic after rmmod softdog (2.6.8.1))

[Chuck Ebbert]

Michael Schierl wrote:

> today, when testing the software watchdog, I accidentally removed the 
> software watchdog kernel module and got a kernel panic.

Does this fix it? (compiled but not tested)

--- linux-2.6.8.1/drivers/char/watchdog/softdog.c.orig  Sun Oct 10 23:08:24 2004
+++ linux-2.6.8.1/drivers/char/watchdog/softdog.c       Sun Oct 10 23:10:12 2004
@@ -135,8 +135,9 @@
 {
        if(test_and_set_bit(0, &timer_alive))
                return -EBUSY;
-       if (nowayout)
-               __module_get(THIS_MODULE);
+
+       __module_get(THIS_MODULE);
+
        /*
         *      Activate timer
         */
@@ -152,6 +153,7 @@
         */
        if (expect_close == 42) {
                softdog_stop();
+               module_put(THIS_MODULE);
        } else {
                printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n");
                softdog_keepalive();


--Chuck Ebbert  11-Oct-04  12:49:45
  Current book: Stephen King: Wizard and Glass

Re: [PATCH] softdog.c (was: Kernel panic after rmmod softdog (2.6.8.1))

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

On Maandag 11 Oktober 2004 19:01, Chuck Ebbert wrote:

> --- linux-2.6.8.1/drivers/char/watchdog/softdog.c.orig  Sun Oct 10 23:08:24 2004
> +++ linux-2.6.8.1/drivers/char/watchdog/softdog.c       Sun Oct 10 23:10:12 2004
> @@ -135,8 +135,9 @@
>  {
>         if(test_and_set_bit(0, &timer_alive))
>                 return -EBUSY;
> -       if (nowayout)
> -               __module_get(THIS_MODULE);
> +
> +       __module_get(THIS_MODULE);
> +
>         /*
>          *      Activate timer
>          */
> @@ -152,6 +153,7 @@
>          */
>         if (expect_close == 42) {
>                 softdog_stop();
> +               module_put(THIS_MODULE);
>         } else {
>                 printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n");
>                 softdog_keepalive();
> 
Now if you do open(), close(), open(), write("V"), close(), the module
becomes unremovable, even without nowayout=1. Isn't is possible to
simply add a softdog_stop() call to watchdog_exit()?

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] softdog.c (was: Kernel panic after rmmod softdog (2.6.8.1))

[Chuck Ebbert]

Arnd Bergmann wrote:

> Now if you do open(), close(), open(), write("V"), close(), the module
> becomes unremovable, even without nowayout=1. Isn't is possible to
> simply add a softdog_stop() call to watchdog_exit()?

  Oh well, at least it can't possibly oops since you can't even remove it.  :)

  How about this instead?

  (Assuming a re-open of the softdog device is allowable; I don't see why not.)

--- linux-2.6.8.1/drivers/char/watchdog/softdog.c.orig  Sun Oct 10 23:08:24 2004
+++ linux-2.6.8.1/drivers/char/watchdog/softdog.c       Wed Oct 13 14:48:20 2004
@@ -82,9 +82,12 @@
 
 static struct timer_list watchdog_ticktock =
                TIMER_INITIALIZER(watchdog_fire, 0, 0);
-static unsigned long timer_alive;
 static char expect_close;
 
+static unsigned long driver_status;
+/* Driver status bits  */
+#define SOFTDOG_TIMER_RUNNING  0
+#define SOFTDOG_DEVICE_OPEN    1
 
 /*
  *     If the timer expires..
@@ -133,9 +136,10 @@
 
 static int softdog_open(struct inode *inode, struct file *file)
 {
-       if(test_and_set_bit(0, &timer_alive))
+       if (test_and_set_bit(SOFTDOG_DEVICE_OPEN, &driver_status))
                return -EBUSY;
-       if (nowayout)
+
+       if ( !test_and_set_bit(SOFTDOG_TIMER_RUNNING, &driver_status))
                __module_get(THIS_MODULE);
        /*
         *      Activate timer
@@ -152,11 +156,13 @@
         */
        if (expect_close == 42) {
                softdog_stop();
+               clear_bit(SOFTDOG_TIMER_RUNNING, &driver_status);
+               module_put(THIS_MODULE);
        } else {
                printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n");
                softdog_keepalive();
        }
-       clear_bit(0, &timer_alive);
+       clear_bit(SOFTDOG_DEVICE_OPEN, &driver_status);
        expect_close = 0;
        return 0;
 }

--Chuck Ebbert  13-Oct-04  19:55:30