Bug 87101 - Problems with SATA Controller and tag ordered submission
Summary: Problems with SATA Controller and tag ordered submission
Status: RESOLVED UNREPRODUCIBLE
Alias: None
Product: IO/Storage
Classification: Unclassified
Component: Serial ATA (show other bugs)
Hardware: i386 Linux
: P1 normal
Assignee: Tejun Heo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-29 00:37 UTC by Ronny.Hegewald
Modified: 2015-05-05 17:57 UTC (History)
2 users (show)

See Also:
Kernel Version: 3.17.1
Tree: Mainline
Regression: Yes


Attachments
Patch 1 (904 bytes, patch)
2015-02-23 23:40 UTC, Ronny.Hegewald
Details | Diff
Patch 2 (1.55 KB, patch)
2015-02-23 23:41 UTC, Ronny.Hegewald
Details | Diff
Patch 3 (2.43 KB, patch)
2015-02-23 23:43 UTC, Ronny.Hegewald
Details | Diff

Description Ronny.Hegewald 2014-10-29 00:37:32 UTC
Hardware is a PCIe SATA Controller with a Sillicon Image 3132 Chipset, 2 eSATA and 2 internal SATA ports. In my configuration only the 2 internal SATA ports are used.

Since commit 8a4aeec8d "libata/ahci: accommodate tag ordered
controllers" the access to the harddisk on the first SATA-port is failing on its first access. The access to the harddisk on the second port is working normal.

When reverting the above commit, access to both harddisks is working fine again.

dmesg output of the failure:

ata5: spurious interrupt (slot_stat 0x8 active_tag 3 sactive 0x0)
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd ec/00:01:00:00:00/00:00:00:00:00/00 tag 3 pio 512 in
         res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5: hard resetting link
ata5: SATA link up 3.0 Gbps (SStatus 123 SControl 0)
ata5: illegal qc_active transition (80000000->00000008)
ata5.00: failed to IDENTIFY (I/O error, err_mask=0x100)
ata5.00: revalidation failed (errno=-5)
ata5: hard resetting link
ata5: SATA link up 3.0 Gbps (SStatus 123 SControl 0)
ata5: illegal qc_active transition (80000000->00000008)
ata5.00: failed to IDENTIFY (I/O error, err_mask=0x100)
ata5.00: revalidation failed (errno=-5)
ata5: limiting SATA link speed to 1.5 Gbps
ata5: hard resetting link 
ata5: SATA link up 1.5 Gbps (SStatus 113 SControl 10)
ata5: illegal qc_active transition (80000000->00000008)
ata5.00: failed to IDENTIFY (I/O error, err_mask=0x100)
ata5.00: revalidation failed (errno=-5)
ata5.00: disabled
ata5: exception Emask 0x2 SAct 0x0 SErr 0x0 action 0x6 frozen t4
ata5: hard resetting link
ata5: SATA link up 1.5 Gbps (SStatus 113 SControl 10)
ata5: EH complete
Comment 1 Tejun Heo 2014-10-29 15:17:37 UTC
Dan, can you please look into this issue?

Thanks.
Comment 2 Dan Williams 2014-10-29 16:16:23 UTC
This is the second controller[1] to have issue with tag order.  Even though they eventually fixed their driver by other means [2], it may be time to make tag order mode opt-in.  I.e. revive the patch to make it opt-out [3] and flip the polarity.  That "other OS" does tag order by default for AHCI, but for other controllers seems we need to be more cautious.

Tejun, any opinion on making it opt-in for AHCI-only vs blacklisting sata_sil to make it opt-out of this behavior?

[1] http://marc.info/?l=linux-kernel&m=140195842716315&w=2
[2] http://marc.info/?l=linux-ide&m=140323674630985&w=2
[3] http://marc.info/?l=linux-ide&m=140206996413716&w=2
Comment 3 Tejun Heo 2014-10-29 16:21:35 UTC
Given that nobody seems to be creating new SATA controller types, I don't think either way would make much difference. I still like the idea of making it an opt-out thing tho as this is such a basic breakage and it'd be difficult to notice unnecessarily disabled cases. It could even be that we're seeing a bug in sata_sil24 not a hardware issue.

Thanks.
Comment 4 Ronny.Hegewald 2015-02-23 23:39:02 UTC
I recently tried kernel 3.18.5 which contains the patch for this issue. But it didn't work, because it turned out that the new flag ATA_FLAG_LOWTAG uses the same bit as SIL24_FLAG_PCIX_IRQ_WOC in sata_sil24. This activates code that is only suppossed to run with Silicon Image 3124 and creates errors with 3132.

The attached 1. patch fixes this problem for the stable-series by changing the used bit for SIL24_FLAG_PCIX_IRQ_WOC. 

The 2. patch for kernel 4.0 fixes this issue by removing ATA_FLAG_LOWTAG and depending code. I have read that this was planned anyway.

But that doesn't fixes the problem alltogether. Having a queue_depth of > 1 creates the same errors as posted in the original report. But as before, this is only a problem on the first internal port, the harddisk on the second port works just fine with NCQ. 

So the problem seems not to be the tag order itself but using anything else then tag 0. The harddisk itself can't be the problem, it works just fine when switched to the second port.

I noticed this problem with NCQ before when i wrote the bug-report in the first place, but couldn't reliably reproduce it and thought it was a seperate issue, so i did not report it back then. But in the meantime it reappeared and is reproduceable every time.

Patch 3 solves this issue by disabling NCQ for the first internal port. This patch is for kernel 4.0 and the stable series.

Interesting behaviour is that if the harddisk is accessed with queue_depth = 1 first after modprobe sata_sil24 (for example by mounting it), then the queue_depth is set back to something bigger then 1 through /sys/block/sdx/device/queue_depth, it survives the access on the harddisk for a short time. For example a ls on a bigger directory it survives ca. 1 second. 

When setting queue_depth back to 1 after this, the access to the harddisk is not possible without a reboot. 

I would assume that setting queue_depth back to 1 would make it accessable again, at least make it possible to remount it. Even smartctl can't access the harddisk anymore to read the SMART-values. 

When the first access on the harddisk is with queue_depth > 1, then it bugs out immediatelly. It can't even mount the harddisk successfully.
Comment 5 Ronny.Hegewald 2015-02-23 23:40:45 UTC
Created attachment 168091 [details]
Patch 1
Comment 6 Ronny.Hegewald 2015-02-23 23:41:57 UTC
Created attachment 168101 [details]
Patch 2
Comment 7 Ronny.Hegewald 2015-02-23 23:43:08 UTC
Created attachment 168111 [details]
Patch 3
Comment 8 Ronny.Hegewald 2015-04-01 20:17:48 UTC
Ping. Anything wrong with this patches?
Comment 9 Tejun Heo 2015-04-06 17:36:20 UTC
LOWTAG flag is removed for upstream already and should be gone after the next merge window. Can you please post the patch relocate the IRQ_WOC to linux-ide@vger.kernel.org w/ proper description and signed-off-by tags? As for disabling NCQ for the first internal port, I'm not sure what's going on. It was quite a while ago but I tested multiple 3132's with NCQ and didn't encounter the problem you're describing, so I'm not sure what's going on in your case.
Comment 10 Ronny.Hegewald 2015-04-09 21:00:38 UTC
(In reply to Tejun Heo from comment #9)
> LOWTAG flag is removed for upstream already and should be gone after the
> next merge window. Can you please post the patch relocate the IRQ_WOC to
> linux-ide@vger.kernel.org w/ proper description and signed-off-by tags? 

I have sent the patch now, i hope it comes through as im not registered on the mailinglist.

> As for disabling NCQ for the first internal port, I'm not sure what's going
> on.
> It was quite a while ago but I tested multiple 3132's with NCQ and didn't
> encounter the problem you're describing, so I'm not sure what's going on in
> your case.

I guess that means that this patch is not acceptable atm. In that case the special handling of tag-order for sata_sil24 could be removed alltogether, as this was only a issue with the first port too. The second port works just fine with tag ordered submission.

Any suggestions how to debug this issue further best? 

Right now i wouldn't exclude some kind of hardware-failure that might only expose itself on the first port. The cabling for both ports is pretty loose, but even more loose on the first port. So maybe the cables don't sit as good on the first port as it should be. But disabling NCQ and tag ordered submission may lower the transfer speed just enough so that signals over the wire are stable enough. 

I have tested the card on 2 different PCs, with different harddisks, different sata-cables, only 1 harddisk attache etc. Always the same failure, and the patches i posted here always fixed the issue. So i think the usual hardware-issues that causes this kind of problems can are not very likely.

So right now i assume some kind of timing-problems, and would start to fiddle with pio_mask and udma_mask settings in the driver next, to slow down operations and to check if it works fine on lower speeds. This and increasing timeouts in the libata error-checking code should be safe for the hardware? 

Coding that close to hardware is not really in my comfort-zone.
Comment 11 Ronny.Hegewald 2015-04-15 22:50:13 UTC
I debugged that issue further and found out why it fails with NCQ.

When commands are send via tag ATA_TAG_INTERNAL, then sata_sil24 changes this internally to tag 0.

But in ata_build_rw_tf the internal commands for ATA_TAG_INTERNAL are explicitly excluded to be send through NCQ. 

Because of this tag mapping in sata_sil24 the internal commands are send via NCQ nonetheless, which seems to cause the problem. 

But why this is only a problem for the first port is still a mystery.

With the following hack and the resolved flag conflict for SIL24_FLAG_PCIX_IRQ_WOC the card seems to work fine with NCQ enabled on both ports. The fifo tag allocator is also still needed.

Im not sure myself how to solve this cleanly, so no proper patch at this time.

--- libata-core.c       
+++ libata-core.c.new   
@@ -752,7 +752,7 @@
        tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
        tf->flags |= tf_flags;
 
-       if (ata_ncq_enabled(dev) && likely(tag != ATA_TAG_INTERNAL)) {
+       if (ata_ncq_enabled(dev) && likely(tag != ATA_TAG_INTERNAL && tag != 0)) {
Comment 12 Tejun Heo 2015-04-17 14:53:08 UTC
Hmmm... I don't get how that would be the case. ata_build_rw_tf() is called before sil24 qc_prep/issue are invoked and EH paths don't issue RW commands. All the above patch achieves is disabling NCQ for all devices. Way back we used to see some timeouts go away when NCQ is disabled or the device is slowed down some other way. This is really sounding like a flaky hardware to me.
Comment 13 Ronny.Hegewald 2015-05-05 17:57:56 UTC
(In reply to Tejun Heo from comment #12)
> Hmmm... I don't get how that would be the case.

Neither can i. It seems that between diving into the specs for the chipset, reading libata-code and trying a lot of different things i managed to get myself confused completely. Sorry for the noise.

I did some more testing and found that the problems only occur when commands are send on tags 3 and 25. Changing libata so that no commands are send to this 2 tags anymore makes the first port work fine with tag ordered submission.

But the 2 tags have different error-behaviours. Tag 3 is the most problematic one. Sending commands to this tag seems to be the reason that the port is finally unaccessable until reboot.

If only tag 3 is excluded from command submissions, then tag 25 is still creating exceptions, but libata can recover from it, finally setting speed to UDMA 100 and disabling NCQ. After that the access to the harddrive works just fine, without any further errors. There are some other differences in their error-behaviours, but i guess its not really important to list them.

In the meantime i also got another 3132 card from another vendor, without the 2 eSata-ports and this one is working without any problems. So it really seems to be a hardware-problem and i will close the bug.

Note You need to log in before you can comment on or make changes to this bug.