[PATCH] touch softlockup watchdog in ide_wait_not_busy

[PATCH] touch softlockup watchdog in ide_wait_not_busy

[Jesper Juhl]

Hi,

This is a resend of a patch I proposed in the 
"[BUG] 2.6.15-rc1, soft lockup detected while probing IDE devices on AMD7441"
thread.
I recieved no ACK/NACK or other feedback on the patch, so I'm resending it in
the hope of getting some comments :)


From: Jesper Juhl <jesper.juhl@gmail.com>

Make sure we touch the softlockup watchdog in 
ide_wait_not_busy() since it may cause the watchdog to trigger, but
there's really no point in that since the loop will eventually return, and
triggering the watchdog won't do us any good anyway.

The  if (!(timeout % 128))  bit is a guess that since 
touch_softlockup_watchdog() is a per_cpu thing it will be cheaper to do the
modulo calculation than calling the function every time through the loop,
especially as the nr of CPU's go up. But it's purely a guess, so I may very 
well be wrong - also, 128 is an arbitrarily chosen value, it's just a nice 
number that'll give us <10 function calls pr second.

Since I have no IDE devices in my box I'm unable to test this beyond making
sure it compiles without warnings or errors (which it does).

Let me know what you think.

Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
---

 drivers/ide/ide-iops.c |    8 ++++++++
 1 files changed, 8 insertions(+)

diff -up linux-2.6.15-rc3-orig/drivers/ide/ide-iops.c linux-2.6.15-rc3/drivers/ide/ide-iops.c
--- linux-2.6.15-rc3-orig/drivers/ide/ide-iops.c	2005-11-29 15:30:32.000000000 +0100
+++ linux-2.6.15-rc3/drivers/ide/ide-iops.c	2005-11-29 15:44:23.000000000 +0100
@@ -24,6 +24,7 @@
 #include <linux/hdreg.h>
 #include <linux/ide.h>
 #include <linux/bitops.h>
+#include <linux/sched.h>
 
 #include <asm/byteorder.h>
 #include <asm/irq.h>
@@ -1243,6 +1244,13 @@ int ide_wait_not_busy(ide_hwif_t *hwif, 
 		 */
 		if (stat == 0xff)
 			return -ENODEV;
+
+		/* 
+		 * We risk triggering the soft lockup detector, but we don't
+		 * want that, so better poke it a bit once in a while.
+		 */
+		if (!(timeout % 128))
+			touch_softlockup_watchdog();
 	}
 	return -EBUSY;
 }

Re: [PATCH] touch softlockup watchdog in ide_wait_not_busy

[Jesper Juhl]

Andrew,

Now that Alexander confirmed this patch fixed his problem, any reason
it couldn't go into -mm ?

He gave this feedback :
On 11/30/05, Alexander V. Inyukhin <shurick@sectorb.msk.ru> wrote:
...
> It seems to work.
> I have no BUG messages during boot with this patch.


/Jesper


On 11/29/05, Jesper Juhl <jesper.juhl@gmail.com> wrote:
> Hi,
>
> This is a resend of a patch I proposed in the
> "[BUG] 2.6.15-rc1, soft lockup detected while probing IDE devices on AMD7441"
> thread.
> I recieved no ACK/NACK or other feedback on the patch, so I'm resending it in
> the hope of getting some comments :)
>
>
> From: Jesper Juhl <jesper.juhl@gmail.com>
>
> Make sure we touch the softlockup watchdog in
> ide_wait_not_busy() since it may cause the watchdog to trigger, but
> there's really no point in that since the loop will eventually return, and
> triggering the watchdog won't do us any good anyway.
>
> The  if (!(timeout % 128))  bit is a guess that since
> touch_softlockup_watchdog() is a per_cpu thing it will be cheaper to do the
> modulo calculation than calling the function every time through the loop,
> especially as the nr of CPU's go up. But it's purely a guess, so I may very
> well be wrong - also, 128 is an arbitrarily chosen value, it's just a nice
> number that'll give us <10 function calls pr second.
>
> Since I have no IDE devices in my box I'm unable to test this beyond making
> sure it compiles without warnings or errors (which it does).
>
> Let me know what you think.
>
> Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
> ---
>
>  drivers/ide/ide-iops.c |    8 ++++++++
>  1 files changed, 8 insertions(+)
>
> diff -up linux-2.6.15-rc3-orig/drivers/ide/ide-iops.c linux-2.6.15-rc3/drivers/ide/ide-iops.c
> --- linux-2.6.15-rc3-orig/drivers/ide/ide-iops.c        2005-11-29 15:30:32.000000000 +0100
> +++ linux-2.6.15-rc3/drivers/ide/ide-iops.c     2005-11-29 15:44:23.000000000 +0100
> @@ -24,6 +24,7 @@
>  #include <linux/hdreg.h>
>  #include <linux/ide.h>
>  #include <linux/bitops.h>
> +#include <linux/sched.h>
>
>  #include <asm/byteorder.h>
>  #include <asm/irq.h>
> @@ -1243,6 +1244,13 @@ int ide_wait_not_busy(ide_hwif_t *hwif,
>                  */
>                 if (stat == 0xff)
>                         return -ENODEV;
> +
> +               /*
> +                * We risk triggering the soft lockup detector, but we don't
> +                * want that, so better poke it a bit once in a while.
> +                */
> +               if (!(timeout % 128))
> +                       touch_softlockup_watchdog();
>         }
>         return -EBUSY;
>  }
>
>
>


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH] touch softlockup watchdog in ide_wait_not_busy

[Bartlomiej Zolnierkiewicz]

On 12/3/05, Jesper Juhl <jesper.juhl@gmail.com> wrote:
> Andrew,
>
> Now that Alexander confirmed this patch fixed his problem, any reason
> it couldn't go into -mm ?

Fine with me now.

Acked-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

> He gave this feedback :
> On 11/30/05, Alexander V. Inyukhin <shurick@sectorb.msk.ru> wrote:
> ...
> > It seems to work.
> > I have no BUG messages during boot with this patch.