Bug 87101

Dan, can you please look into this issue?

Thanks.

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

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.

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.

Created attachment 168091 [details]
Patch 1

Created attachment 168101 [details]
Patch 2

Created attachment 168111 [details]
Patch 3

Ping. Anything wrong with this patches?

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.

(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.

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)) {

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.

(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.