SATA exceptions with 2.6.20-rc5

PATCH New Robert Hancock 3 participants 9 messages Jan 23, 2007 – Feb 9, 2007 6 messages not in the archive libata

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): fa.fkPTbUGmKc/[email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

Robert Hancock

author

2007-01-23 23:20 UTC #6 lore raw

Quoted 1 line · Larry Walton

The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.

Can any of the rest of you that have been seeing this problem also confirm that this fixes it?

Signature · 3 lines

Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/

Björn Steinbrink

2007-01-24 00:39 UTC #7 lore raw

Quoted 7 lines · Robert Hancock

Quoted 3 lines · Larry Walton
The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.
Can any of the rest of you that have been seeing this problem also confirm that this fixes it?

Seems to work for me, uptime is about an hour now and no exception yet. Had the stress test running for only about 10 minutes, but I usually got an exception within an hour even during plain irssi usage, so I'm quite confident that the patch fixes it.

Thanks,
Björn

Robert Hancock

author

2007-01-24 02:09 UTC #8 lore raw

PATCH 1/1 sata_nv: don't rely on NV_INT_DEV indication with ADMA

OK, here it is in full signed-off glory. Hopefully we can get this in for 2.6.20.

---

Several people reported issues with certain drive commands timing out on sata_nv controllers running in ADMA mode. The commands in question were non-DMA-mapped commands, usually FLUSH CACHE or FLUSH CACHE EXT.

From experimentation it appears that the NV_INT_DEV indication isn't always set when a legitimate command completion interrupt is received on a legacy-mode command, at least not on these controllers in ADMA mode. When a command is pending on the port, force the flag on always in the irq_stat value before calling nv_host_intr so that the drive busy state is always checked by ata_host_intr.

This also fixes some questionable code in nv_host_intr which called ata_check_status when a command was pending and ata_host_intr returned "unhandled". If the device interrupted at just the wrong time this could cause interrupts to be lost.

Signed-off-by Robert Hancock <[email protected]>

drivers/ata/sata_nv.c +6 -8

@@ -700,7 +700,6 @@ static void nv_adma_check_cpb(struct ata
 static int nv_host_intr(struct ata_port *ap, u8 irq_stat)
 {
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
-	int handled;
 
 	/* freeze if hotplugged */
 	if (unlikely(irq_stat & (NV_INT_ADDED | NV_INT_REMOVED))) {

@@ -719,13 +718,7 @@ static int nv_host_intr(struct ata_port 
 	}
 
 	/* handle interrupt */
-	handled = ata_host_intr(ap, qc);
-	if (unlikely(!handled)) {
-		/* spurious, clear it */
-		ata_check_status(ap);
-	}
-
-	return 1;
+	return ata_host_intr(ap, qc);
 }
 
 static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)

@@ -752,6 +745,11 @@ static irqreturn_t nv_adma_interrupt(int
 			if (pp->flags & NV_ADMA_PORT_REGISTER_MODE) {
 				u8 irq_stat = readb(host->mmio_base + NV_INT_STATUS_CK804)
 					>> (NV_INT_PORT_SHIFT * i);
+				if(ata_tag_valid(ap->active_tag))
+					/** NV_INT_DEV indication seems unreliable at times
+					    at least in ADMA mode. Force it on always when a
+					    command is active, to prevent losing interrupts. */
+					irq_stat |= NV_INT_DEV;
 				handled += nv_host_intr(ap, irq_stat);
 				continue;
 			}

Björn Steinbrink

2007-02-03 01:42 UTC #9 lore raw

In reply to Björn Steinbrink

Quoted 6 lines · Björn Steinbrink

Quoted 5 lines · Robert Hancock
Quoted 3 lines · Larry Walton
The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.

Quoted 7 lines

Quoted 2 lines
Can any of the rest of you that have been seeing this problem also confirm that this fixes it?
Seems to work for me, uptime is about an hour now and no exception yet. Had the stress test running for only about 10 minutes, but I usually got an exception within an hour even during plain irssi usage, so I'm quite confident that the patch fixes it.

Or maybe not :( Just got an exception on 2.6.20-rc6. Took 4 days of uptime to trigger, so it's just a lot harder to trigger now.

Björn

Robert Hancock

author

2007-02-03 05:48 UTC #10 lore raw

Quoted 4 lines · Björn Steinbrink

Quoted 3 lines · Björn Steinbrink
Quoted 2 lines · Robert Hancock
Quoted 1 line · Larry Walton
The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Can any of the rest of you that have been seeing this problem also confirm that this fixes it?
Seems to work for me, uptime is about an hour now and no exception yet. Had the stress test running for only about 10 minutes, but I usually got an exception within an hour even during plain irssi usage, so I'm quite confident that the patch fixes it.
Or maybe not :( Just got an exception on 2.6.20-rc6. Took 4 days of uptime to trigger, so it's just a lot harder to trigger now.

Same exception details as before?

There's a patch in -mm (sata_nv-use-adma-for-nodata-commands.patch) which should hopefully avoid this problem for the cache flush commands, at least - can you try that one out? You'll have to apply the other sata_nv patches in -mm first, i.e. this order:

http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-cleanup-adma-error-handling-v2.patch
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-cleanup-adma-error-handling-v2-cleanup.patch
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-use-adma-for-nodata-commands.patch

Signature · 3 lines

Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/

Björn Steinbrink

2007-02-04 01:13 UTC #11 lore raw

Quoted 14 lines · Robert Hancock

Quoted 13 lines · Björn Steinbrink
Quoted 11 lines · Björn Steinbrink
Quoted 6 lines · Robert Hancock
Quoted 3 lines · Larry Walton
The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.
Can any of the rest of you that have been seeing this problem also confirm that this fixes it?
Seems to work for me, uptime is about an hour now and no exception yet. Had the stress test running for only about 10 minutes, but I usually got an exception within an hour even during plain irssi usage, so I'm quite confident that the patch fixes it.

Quoted 4 lines

Quoted 2 lines
Or maybe not :( Just got an exception on 2.6.20-rc6. Took 4 days of uptime to trigger, so it's just a lot harder to trigger now.
Same exception details as before?

Yes, exactly the same.

Quoted 8 lines

There's a patch in -mm (sata_nv-use-adma-for-nodata-commands.patch) which should hopefully avoid this problem for the cache flush commands, at least - can you try that one out? You'll have to apply the other sata_nv patches in -mm first, i.e. this order:
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-cleanup-adma-error-handling-v2.patch
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-cleanup-adma-error-handling-v2-cleanup.patch
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-use-adma-for-nodata-commands.patch

Got 2.6.20-rc7 with them applied now (the rejects seemed trivial enough for me to fix them). Let's see how that works out...

Björn

Björn Steinbrink

2007-02-09 12:03 UTC #12 lore raw

Quoted 6 lines · Björn Steinbrink

Quoted 5 lines · Robert Hancock
There's a patch in -mm (sata_nv-use-adma-for-nodata-commands.patch) which should hopefully avoid this problem for the cache flush commands, at least - can you try that one out? You'll have to apply the other sata_nv patches in -mm first, i.e. this order:

Quoted 6 lines

Quoted 3 lines
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-cleanup-adma-error-handling-v2.patch
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-cleanup-adma-error-handling-v2-cleanup.patch
http://www2.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/sata_nv-use-adma-for-nodata-commands.patch
Got 2.6.20-rc7 with them applied now (the rejects seemed trivial enough for me to fix them). Let's see how that works out...

After about 1.5 days of uptime, an involuntary reboot and another 3 days of uptime, no sign of an exception. No stress testing was done, but a few disk intensive actions did happen, at least more than with that -rc6 that did throw an exception at me.

Björn

Ian Kumlien

2007-01-24 08:24 UTC #13 lore raw

In reply to Robert Hancock

Quoted 7 lines · Robert Hancock

Quoted 3 lines · Larry Walton
The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.
Can any of the rest of you that have been seeing this problem also confirm that this fixes it?

I applied it yesterday and today my dmesg contains three:

BUG: at mm/truncate.c:60 cancel_dirty_page()

Call Trace:
 [<ffffffff8029f3e5>] cancel_dirty_page+0x43/0x71
 [<ffffffff802ec1ab>] reiserfs_cut_from_item+0x5f8/0x61d
 [<ffffffff802074fc>] find_get_page+0x21/0x47
 [<ffffffff802ec51d>] reiserfs_do_truncate+0x34d/0x495
 [<ffffffff802d9d47>] reiserfs_truncate_file+0x199/0x2aa
 [<ffffffff802df9c5>] reiserfs_file_release+0x261/0x281
 [<ffffffff80211b02>] __fput+0xb1/0x17d
 [<ffffffff802218e0>] filp_close+0x5d/0x65
 [<ffffffff8021bef5>] sys_close+0x8c/0xcf
 [<ffffffff8025725e>] system_call+0x7e/0x83

Which never happened before... I dunno if they are related though, but they weren't there before...

(It does fix the timeout problem)

Signature · 1 line

Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net

Björn Steinbrink

2007-01-24 14:41 UTC #14 lore raw

Quoted 11 lines · Ian Kumlien

Quoted 7 lines · Robert Hancock
Quoted 3 lines · Larry Walton
The last patch (sata_nv-force-int-dev-in-interrupt.patch) seems to have fix the problem. Much appreciated, thank you. I'd consider it a must have in 2.6.20.
Can any of the rest of you that have been seeing this problem also confirm that this fixes it?
I applied it yesterday and today my dmesg contains three:
BUG: at mm/truncate.c:60 cancel_dirty_page()

David Chinner sent two patches regarding that bug yesterday.
http://lkml.org/lkml/2007/1/23/190
http://lkml.org/lkml/2007/1/23/192

Björn