Skip to contentSkip to conversation

sata_nv hotplug in 2.6.18

PATCH In review Jim Paris 3 participants 7 messages Sep 27, 2006 – Nov 1, 2006 libata

lore
Share & tools
lore

Jim Paris

author
#0 lore raw

Hi Tejun,

I set up a new machine with a NVIDIA SATA controller and 2.6.18. Hotplug did not work correctly. I discovered that it's the same problem Philipp Wagner had a few months ago with 2.6.18-rc4:

http://www.spinics.net/lists/linux-ide/msg04081.html

The patch you provided there solves the problem on my system as well. I haven't seen this patch mentioned anywhere else, and I don't see it in libata-dev.git, so I just wanted to check on its status.

Relevant logs below.

Thanks,
-jim

######

Affected device:

00:07.0 IDE interface: nVidia Corporation CK804 Serial ATA Controller (rev f3) (prog-if 85 [Master SecO PriO])

        Subsystem: Tyan Computer Unknown device 2877
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 0 (750ns min, 250ns max)
        Interrupt: pin A routed to IRQ 58
        Region 0: I/O ports at 1440 [size=8]
        Region 1: I/O ports at 1434 [size=4]
        Region 2: I/O ports at 1438 [size=8]
        Region 3: I/O ports at 1430 [size=4]
        Region 4: I/O ports at 1410 [size=16]
        Region 5: Memory at d8002000 (32-bit, non-prefetchable) [size=4K]
        Capabilities: [44] Power Management version 2
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-

######

Without patch:

Removed disk:

[  298.452420] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1910000 action 0x2 frozen
[  299.169503] ata3: soft resetting port
[  299.169509] ata3: SATA link down (SStatus 0 SControl 300)
[  299.169514] ata3: failed to recover some devices, retrying in 5 secs
[  304.171105] ata3: hard resetting port
[  304.894753] ata3: SATA link down (SStatus 0 SControl 300)
[  304.894758] ata3: failed to recover some devices, retrying in 5 secs
[  309.896354] ata3: hard resetting port
[  310.620003] ata3: SATA link down (SStatus 0 SControl 300)
[  310.620010] ata3.00: disabled
[  311.123763] ata3: EH complete
[  311.123774] ata3.00: detaching (SCSI 2:0:0:0)

Plugged disk back in:

[  352.429871] ata3: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2 frozen
[  360.148230] ata3: port is slow to respond, please be patient
[  383.153148] ata3: port failed to respond (30 secs)
[  383.153198] ata3: soft resetting port
[  390.309735] ata3: port is slow to respond, please be patient
[  413.318647] ata3: port failed to respond (30 secs)
[  413.318705] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  413.318712] ata3: EH complete

The disk never comes back.

######

With patch:

Removed disk:

[  132.414496] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1810000 action 0x2 frozen
[  132.925157] ata3: hard resetting port
[  133.648811] ata3: SATA link down (SStatus 0 SControl 300)
[  133.648818] ata3: failed to recover some devices, retrying in 5 secs
[  139.162162] ata3: hard resetting port
[  139.885815] ata3: SATA link down (SStatus 0 SControl 300)
[  139.885821] ata3: failed to recover some devices, retrying in 5 secs
[  145.399165] ata3: hard resetting port
[  146.122816] ata3: SATA link down (SStatus 0 SControl 300)
[  146.122821] ata3.00: disabled
[  146.626577] ata3: EH pending after completion, repeating EH (cnt=4)
[  146.626590] ata3: EH complete
[  146.626597] ata3.00: detaching (SCSI 2:0:0:0)

Plugged disk back in:

[  174.511797] ata3: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2 frozen
[  175.024925] ata3: waiting for device to spin up (8 secs)
[  175.027704] ata3: hard resetting port
[  181.621773] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  181.622628] ata3.00: ATA-7, max UDMA/133, 625142448 sectors: LBA48 NCQ (depth 0/32)
[  181.623677] ata3.00: configured for UDMA/133
[  181.623681] ata3: EH pending after completion, repeating EH (cnt=4)
[  181.623691] ata3: EH complete
[  181.623810]   Vendor: ATA       Model: ST3320620AS       Rev: 3.AA
[  181.623821]   Type:   Direct-Access                      ANSI SCSI revision: 05
[  181.623910] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  181.623923] sdc: Write Protect is off
[  181.623926] sdc: Mode Sense: 00 3a 00 00
[  181.623946] SCSI device sdc: drive cache: write back
[  181.624005] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  181.624017] sdc: Write Protect is off
[  181.624020] sdc: Mode Sense: 00 3a 00 00
[  181.624039] SCSI device sdc: drive cache: write back
[  181.624044]  sdc: sdc1
[  181.636017] sd 2:0:0:0: Attached scsi disk sdc

Everything is fine.

#1 lore raw

PATCH 1/1 libata: lengthen interval between SRST set and clear

20us isn't enough for some SATA controllers (sata_nv) ending up not sending the second FIS27 to clear SRST. This usually results in SRST timeout causing excessive delays during hotplug.

This patch lengthens the delay from 20us to 1ms and also converts it to msleep() instead of busy wait.

Signed-off-by Tejun Heo <[email protected]>

---
Hello, Jim Paris, Philipp Wagner.

Can you guys please give this patch a shot? This patch is againt libata-dev#upstream so if you're using 2.6.18, the patched file (libata-core.c) wouldn't be there. If that's the case, just edit drivers/scsi/libata-core.c accordng to the following patch (replace udelay(20)s in ata_bus_softreset() w/ msleep(1)).

Jeff, this function is called from both old EH and new EH's ata_std_softreset(). I think this change is safe but curious about your opinion.

Thanks.

drivers/ata/libata-core.c +4 -4
index 753b015..007020e 100644
@@ -2414,15 +2414,15 @@ static unsigned int ata_bus_softreset(st
 	/* software reset.  causes dev0 to be selected */
 	if (ap->flags & ATA_FLAG_MMIO) {
 		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
-		udelay(20);	/* FIXME: flush */
+		msleep(1);	/* FIXME: flush */
 		writeb(ap->ctl | ATA_SRST, (void __iomem *) ioaddr->ctl_addr);
-		udelay(20);	/* FIXME: flush */
+		msleep(1);	/* FIXME: flush */
 		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
 	} else {
 		outb(ap->ctl, ioaddr->ctl_addr);
-		udelay(10);
+		msleep(1);
 		outb(ap->ctl | ATA_SRST, ioaddr->ctl_addr);
-		udelay(10);
+		msleep(1);
 		outb(ap->ctl, ioaddr->ctl_addr);
 	}
author
#2 lore raw

Re: [PATCH] libata: lengthen interval between SRST set and clear

Hi Tejun,

Quoted 6 lines

20us isn't enough for some SATA controllers (sata_nv) ending up not sending the second FIS27 to clear SRST. This usually results in SRST timeout causing excessive delays during hotplug.

This patch lengthens the delay from 20us to 1ms and also converts it to msleep() instead of busy wait.

Thanks for the patch. Unfortunately it doesn't seem to help on 2.6.18:

Unplug:

[  190.559849] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1810000 action 0x2 frozen
[  191.274576] ata3: soft resetting port
[  191.274580] ata3: SATA link down (SStatus 0 SControl 300)
[  191.274585] ata3: failed to recover some devices, retrying in 5 secs
[  196.276276] ata3: hard resetting port
[  196.999941] ata3: SATA link down (SStatus 0 SControl 300)
[  196.999947] ata3: failed to recover some devices, retrying in 5 secs
[  202.001639] ata3: hard resetting port
[  202.725303] ata3: SATA link down (SStatus 0 SControl 300)
[  202.725309] ata3.00: disabled
[  203.229080] ata3: EH complete
[  203.229091] ata3.00: detaching (SCSI 2:0:0:0)

Replug:

[  222.501022] ata3: exception Emask 0x10 SAct 0x0 SErr 0x150000 action 0x2 frozen
[  230.220666] ata3: port is slow to respond, please be patient
[  253.226040] ata3: port failed to respond (30 secs)
[  253.226090] ata3: soft resetting port
[  260.398768] ata3: port is slow to respond, please be patient
[  283.404138] ata3: port failed to respond (30 secs)
[  283.404195] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  283.404203] ata3: EH complete

and the disk never comes back. This is just like my earlier log from vanilla 2.6.18 (except SErr differs by 0x100000).

-jim

#3 lore raw

Re: [PATCH] libata: lengthen interval between SRST set and clear

Quoted 10 lines · Jim Paris

Hi Tejun,

Quoted 6 lines

20us isn't enough for some SATA controllers (sata_nv) ending up not sending the second FIS27 to clear SRST. This usually results in SRST timeout causing excessive delays during hotplug.

This patch lengthens the delay from 20us to 1ms and also converts it to msleep() instead of busy wait.

Thanks for the patch. Unfortunately it doesn't seem to help on 2.6.18:

I see.

Justing, the solution doesn't seem to work. Have you guys verified that the controller doesn't send the second H2D regsiter FIS clearing SRST w/ 20us delay? If you guys did, the patch needs to make into tree whether it helps this particular case or not. If not, I think it's best to leave it alone.

Jim, can you please test the following patch? It only adds ATA_FLAG_HRST_TO_RESUME.

drivers/ata/sata_nv.c +6 -3
index 8cd730f..647e7b8 100644
@@ -257,7 +257,8 @@ static struct ata_port_info nv_port_info
 	/* generic */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,

@@ -266,7 +267,8 @@ static struct ata_port_info nv_port_info
 	/* nforce2/3 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,

@@ -275,7 +277,8 @@ static struct ata_port_info nv_port_info
 	/* ck804 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
author
#4 lore raw

Re: [PATCH] libata: lengthen interval between SRST set and clear

Quoted 2 lines · Tejun Heo

Jim, can you please test the following patch? It only adds ATA_FLAG_HRST_TO_RESUME.

Yes, just ATA_FLAG_HRST_TO_RESUME also works fine:

Unplug:

[  220.354520] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1810000 action 0x2 frozen
[  220.354591] ata3: hard resetting port
[  221.075828] ata3: SATA link down (SStatus 0 SControl 300)
[  221.075833] ata3: failed to recover some devices, retrying in 5 secs
[  226.080467] ata3: hard resetting port
[  226.804116] ata3: SATA link down (SStatus 0 SControl 300)
[  226.804121] ata3: failed to recover some devices, retrying in 5 secs
[  231.805716] ata3: hard resetting port
[  232.529366] ata3: SATA link down (SStatus 0 SControl 300)
[  232.529373] ata3.00: disabled
[  233.033123] ata3: EH pending after completion, repeating EH (cnt=4)
[  233.033133] ata3: EH complete
[  233.033141] ata3.00: detaching (SCSI 2:0:0:0)

Replug:

[  247.048727] ata3: exception Emask 0x10 SAct 0x0 SErr 0x150000 action 0x2 frozen
[  247.048798] ata3: hard resetting port
[  253.867127] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  253.867930] ata3.00: ATA-7, max UDMA/133, 625142448 sectors: LBA48 NCQ (depth 0/32)
[  253.868982] ata3.00: configured for UDMA/133
[  253.868986] ata3: EH pending after completion, repeating EH (cnt=4)
[  253.868994] ata3: EH complete
[  253.869342]   Vendor: ATA       Model: ST3320620AS       Rev: 3.AA
[  253.869351]   Type:   Direct-Access                      ANSI SCSI revision: 05
[  253.869507] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  253.869737] sdc: Write Protect is off
[  253.869740] sdc: Mode Sense: 00 3a 00 00
[  253.869931] SCSI device sdc: drive cache: write back
[  253.870082] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  253.870189] sdc: Write Protect is off
[  253.870192] sdc: Mode Sense: 00 3a 00 00
[  253.870372] SCSI device sdc: drive cache: write back
[  253.870378]  sdc: sdc1
[  253.887285] sd 2:0:0:0: Attached scsi disk sdc

Thanks,
-jim

#5 lore raw

PATCH 1/1 sata_nv: SRST sometimes fails after hotplug, use HRST_TO_RESUME

NV controllers sometimes fail to perform softreset after hotplug. Make it use hardreset to resume link.

Signed-off-by Tejun Heo <[email protected]>
drivers/ata/ahci.c +0 -0
drivers/ata/sata_nv.c +6 -3
index 8cd730f..647e7b8 100644
@@ -257,7 +257,8 @@ static struct ata_port_info nv_port_info
 	/* generic */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,

@@ -266,7 +267,8 @@ static struct ata_port_info nv_port_info
 	/* nforce2/3 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,

@@ -275,7 +277,8 @@ static struct ata_port_info nv_port_info
 	/* ck804 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
Applied
#6 lore raw

Re: [PATCH] sata_nv: SRST sometimes fails after hotplug, use HRST_TO_RESUME

Quoted 4 lines · Tejun Heo

NV controllers sometimes fail to perform softreset after hotplug. Make it use hardreset to resume link.

Signed-off-by Tejun Heo <[email protected]>

applied