STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system suspend

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Please feel free to suggest what can possibly be the cause, thanks.

Quoted 17 lines · Aaron Lu

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Quoted 1 line

Please feel free to suggest what can possibly be the cause, thanks.

You really need to discriminate better between conditions we should and shouldn't care about for suspend. so in sd_suspend, we definitely care if we can't flush the cache of a write back device because the power off could lose data. We don't really care if the disk says I can't stop, so this is probably the correct fix.

---

drivers/scsi/sd.c +5 -1

index 7992635..384b621d 100644
@@ -3079,7 +3079,11 @@ static int sd_suspend(struct device *dev)
 
  	if (sdkp->device->manage_start_stop) {
  		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
-		ret = sd_start_stop_device(sdkp, 0);
+		/*
+		 * this is informational for the disk we're going to power it
+		 * off anyway, so don't bother about the return status
+		 */
+		sd_start_stop_device(sdkp, 0);
  	}
 
  done:

Quoted 18 lines · James Bottomley

Quoted 17 lines · Aaron Lu

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Quoted 7 lines

Quoted 1 line

Please feel free to suggest what can possibly be the cause, thanks.

You really need to discriminate better between conditions we should and shouldn't care about for suspend. so in sd_suspend, we definitely care if we can't flush the cache of a write back device because the power off could lose data. We don't really care if the disk says I can't stop, so this is probably the correct fix.

Yes, this will make suspend succeed, but the problem is still there - the drive does not respond to this command, while it will if async suspend for the target device is disabled. I don't think scsi code has any fault here, it feels more like a chip bug to me.

And if we solve it this way, affected user may complain about long delay when suspending or uncomfortable with the error messages in dmesg, since that command would eventually timeout. So I hope we can nderstand what is going wrong here and perhaps do something to avoid it.

Oh, that seems to be the suspend order isn't careful enough. __device_suspend() waits for its children, but the host disk are too far separated in the device tree. If the immediate children of the host are all sync, that wait never actually waits for anything.

Quoted 4 lines · James Bottomley

Oh, that seems to be the suspend order isn't careful enough. __device_suspend() waits for its children, but the host disk are too far separated in the device tree. If the immediate children of the host are all sync, that wait never actually waits for anything.

I was going to make exactly this same point. During async suspend, the PM core is careful to make sure that no device is suspended before its children. But there aren't any other checks, so if device A isn't an ancestor of device B then it's possible for async suspend to power down A before B. This can cause problems if B needs A to be active while B is suspending.

Does the ATA system have any non-ancestor dependencies like this? If it does, the appropriate driver can be fixed to take them into account.

Hi James and Alan,

Quoted 7 lines · Alan Stern

Quoted 4 lines · James Bottomley

Oh, that seems to be the suspend order isn't careful enough. __device_suspend() waits for its children, but the host disk are too far separated in the device tree. If the immediate children of the host are all sync, that wait never actually waits for anything.

Quoted 6 lines

I was going to make exactly this same point. During async suspend, the PM core is careful to make sure that no device is suspended before its children. But there aren't any other checks, so if device A isn't an ancestor of device B then it's possible for async suspend to power down A before B. This can cause problems if B needs A to be active while B is suspending.

Thanks for the suggestions.

Quoted 3 lines

Does the ATA system have any non-ancestor dependencies like this? If it does, the appropriate driver can be fixed to take them into account.

I don't think there is, and the relationship is like this:

ata_host_controller* (named sata_nv xxx)
|
ata_port* (named atax, while "ata_port atax" is another device)
/ \
scsi_host ata_link

    |		   |
scsi_target	ata_device
    |

scsi_device* (named sd x:x:x:x)

With the devices that have actual PM operation functions defined have the asterisk next to it.

So ata_host_controller waits for all of the ata_ports, and the ata_port waits for both scsi_host and ata_link. scsi_host waits for scsi_target, and scsi_target waits for scsi_device. So if scsi_device is not done, ata_port will not start. Doesn't look like a problem to me.

And from the log:
https://bugzilla.kernel.org/attachment.cgi?id=95101
It also looks like the order is correct.

Quoted 38 lines · Aaron Lu

Hi James and Alan,

Quoted 7 lines · Alan Stern

Quoted 4 lines · James Bottomley

Oh, that seems to be the suspend order isn't careful enough. __device_suspend() waits for its children, but the host disk are too far separated in the device tree. If the immediate children of the host are all sync, that wait never actually waits for anything.

Quoted 6 lines

I was going to make exactly this same point. During async suspend, the PM core is careful to make sure that no device is suspended before its children. But there aren't any other checks, so if device A isn't an ancestor of device B then it's possible for async suspend to power down A before B. This can cause problems if B needs A to be active while B is suspending.

Thanks for the suggestions.

Quoted 3 lines

Does the ATA system have any non-ancestor dependencies like this? If it does, the appropriate driver can be fixed to take them into account.

I don't think there is, and the relationship is like this:

ata_host_controller* (named sata_nv xxx)
|
ata_port* (named atax, while "ata_port atax" is another device)
/ \
scsi_host ata_link

     |		   |
scsi_target	ata_device
     |

scsi_device* (named sd x:x:x:x)

With the devices that have actual PM operation functions defined have the asterisk next to it.

Quoted 8 lines

So ata_host_controller waits for all of the ata_ports, and the ata_port waits for both scsi_host and ata_link. scsi_host waits for scsi_target, and scsi_target waits for scsi_device. So if scsi_device is not done, ata_port will not start. Doesn't look like a problem to me.

And from the log:
https://bugzilla.kernel.org/attachment.cgi?id=95101
It also looks like the order is correct.

That's not what that log appears to say. Here are the relevant bits

[ 7377.813634] sd 2:0:0:0: async_suspend: scheduled
[ 7377.813636] sd 2:0:0:0: __device_suspend: starts
[ 7377.813639] sd 2:0:0:0: [sdb] Synchronizing SCSI cache
... so now we've begun suspend]
[ 7377.813750] sd 2:0:0:0: [sdb] Stopping disk
[... here we send STANDBY IMMEDIATE ]
[ 7378.237627] sata_nv 0000:00:05.2: async_suspend: scheduled
[ 7378.237631] sata_nv 0000:00:05.2: __device_suspend: starts
[... we begin to shut down the host ]
[ 7378.249333] sata_nv 0000:00:05.2: __device_suspend: done

[... host shutdown complete ]

[ 7408.372642] ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
[ 7408.372647] ata3.00: failed command: STANDBY IMMEDIATE
[ ... command times out ]
[ 7408.870675] dpm_run_callback(): scsi_bus_suspend+0x0/0x20 [scsi_mod] returns 134217730
[ 7408.870681] sd 2:0:0:0: __device_suspend: done

We shut down the host controller before the command completed. This appears to cause the timeout

James

So ata_host_controller waits for all of the ata_ports, and the ata_port waits for both scsi_host and ata_link. scsi_host waits for scsi_target, and scsi_target waits for scsi_device. So if scsi_device is not done, ata_port will not start. Doesn't look like a problem to me.

And from the log:
https://bugzilla.kernel.org/attachment.cgi?id=95101
It also looks like the order is correct.

Quoted 11 lines · Aaron Lu

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

Quoted 7 lines

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Please feel free to suggest what can possibly be the cause, thanks.

I sometimes despair of people getting PM stuff right. What on earth is the point of refusing to suspend if the disk refuses to stop? In theory it gives the device more time to park its head, but almost no modern drive requires this. The next action suspend will take (if allowed) is to power down peripherals which will forcibly stop the device. The stop request is purely informational for the device. If it ignores it, then the bigger hammer still works.

You really need to discriminate better between conditions we should and shouldn't care about for suspend. so in sd_suspend, we definitely care if we can't flush the cache of a write back device because the power off could lose data. We don't really care if the disk says I can't stop, so this is probably the correct fix.

James

---

drivers/scsi/sd.c +5 -1

index 7992635..384b621d 100644
@@ -3079,7 +3079,11 @@ static int sd_suspend(struct device *dev)
 
        if (sdkp->device->manage_start_stop) {
                sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
-               ret = sd_start_stop_device(sdkp, 0);
+               /*
+                * this is informational for the disk we're going to power it
+                * off anyway, so don't bother about the return status
+                */
+               sd_start_stop_device(sdkp, 0);
        }
 
 done:

(resending due to GMail mess-up, sorry)

Quoted 20 lines · James Bottomley

Quoted 11 lines · Aaron Lu

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

Quoted 7 lines

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Please feel free to suggest what can possibly be the cause, thanks.

Quoted 7 lines

I sometimes despair of people getting PM stuff right. What on earth is the point of refusing to suspend if the disk refuses to stop? In theory it gives the device more time to park its head, but almost no modern drive requires this. The next action suspend will take (if allowed) is to power down peripherals which will forcibly stop the device. The stop request is purely informational for the device. If it ignores it, then the bigger hammer still works.

This really does matter. Especially on laptop hard drives (but on many desktop ones as well), we really want to stop the drive, and therefore unload the heads, before the power is shut off. Drives are often rated for a much lower number of emergency head unloads (caused by power loss) over their lifespan than normal software-commanded ones. So over a long period of time, repeatedly failing to stop the drive before power off will shorten its life. Maybe aborting suspend for this is a bit harsh but it's not something that should be ignored.

Just to add to this point, it doesn't just matter for rotational drives, either. A lot of SSDs use the "standby now" command to do some kind of cleanup before power off. Some drives document that a subsequent power up may take longer for the drive to be ready if it wasn't "spun down" prior to the last power off. It's conceivable that in some crappy drives, lack of proper power-off sequence could even cause data loss.

Quoted 32 lines · Robert Hancock

(resending due to GMail mess-up, sorry)

Quoted 20 lines · James Bottomley

Quoted 11 lines · Aaron Lu

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

Quoted 7 lines

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Please feel free to suggest what can possibly be the cause, thanks.

Quoted 7 lines

I sometimes despair of people getting PM stuff right. What on earth is the point of refusing to suspend if the disk refuses to stop? In theory it gives the device more time to park its head, but almost no modern drive requires this. The next action suspend will take (if allowed) is to power down peripherals which will forcibly stop the device. The stop request is purely informational for the device. If it ignores it, then the bigger hammer still works.

Quoted 8 lines

This really does matter. Especially on laptop hard drives (but on many desktop ones as well), we really want to stop the drive, and therefore unload the heads, before the power is shut off. Drives are often rated for a much lower number of emergency head unloads (caused by power loss) over their lifespan than normal software-commanded ones. So over a long period of time, repeatedly failing to stop the drive before power off will shorten its life. Maybe aborting suspend for this is a bit harsh but it's not something that should be ignored.

Where do you get this information from? The only smart parameter tracking this is the power off retract count (it may originally have been the emergency head retract count, but it was renamed a while ago), and that happens for any head unload, however caused. I know SCSI devices long ago ceased caring about this, because the in-drive capacitance is sufficient to achieve a head unload before the device completely spins down in forced power off (I admit the really old IDE devices ... the ones that required the OS to do everything did have a nasty habit of crashing their heads onto the surface, but they stopped being manufactured years ago) ... I really don't see how any modern SATA device would fail to do this.

Quoted 8 lines

Just to add to this point, it doesn't just matter for rotational drives, either. A lot of SSDs use the "standby now" command to do some kind of cleanup before power off. Some drives document that a subsequent power up may take longer for the drive to be ready if it wasn't "spun down" prior to the last power off. It's conceivable that in some crappy drives, lack of proper power-off sequence could even cause data loss.

Well, no, they shouldn't ... not unless the drive violates its data integrity commitment, which is going to cause a whole load of FS corruption. All our Jornalled FS guarantees rely on this data integrity commitment. If they're violated, we have a whole boat load of data safety issues.

I'm sure there are some SSDs that do violate their data integrity commitments - a while ago some tests were done (don't have a link handy and I don't think they identified the actual vendor/model of the drives in any case) but there were definitely some SSDs that did do nasty things like trash unrelated data if power was lost while writing, etc. So we definitely don't want to risk this sort of thing occurring on a normal power-off or suspend.

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Please feel free to suggest what can possibly be the cause, thanks.

-Aaron

Hello,

Sorry that i have not had time to investigate further.

However, it would seem that the logs provided by bladud shows (as i see it) the problem (the logs Aaron requested).

https://bugzilla.kernel.org/show_bug.cgi?id=48951

1) When running without async suspend, the disks are suspended *after* the sata_nv module. That makes sense, as we cannot expect the controller to handle commands after it has been suspended.

2) when running with async suspended, the disks are suspended *before* the sata_nv module. This does not make much sense. This would also explain certain strangeness (different outcome) in the bug, as effectively the disks are racing against their own controller!

Hmmmm how is this supposed to work? Even though async suspend is enabled, the kernel must still have some dependencies that describe in which order things can be suspended. What is the correct way of telling the kernel (PM) that sata_nv cannot be suspended before all child disks are suspended? How do other drivers handle this?

Any comments?

/pedro

Quoted 21 lines · Aaron Lu

Hi all,

I've seen some reports on STANDBY IMMEDIATE failed on NVIDIA MCP5x controllers when system goes to suspend(this command is sent by scsi sd driver on system suspend as a SCSI STOP command, which is translated to STANDBY IMMEDIATE ATA command). I've no idea of why this happened, so I wrote this email in hope of getting some new idea.

The related bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=48951

And google search showed that Peter reported a similar problem here:
http://marc.info/?l=linux-ide&m=133534061316338&w=2

And bladud has found that, disable asyn suspend for the scsi target device can work around this problem.

Please feel free to suggest what can possibly be the cause, thanks.

-Aaron

The device that is responsible for PM operations for the disk is named something like sd X:0:0:0, where X is the host number, and is normally attached under the ata port number+1. So sd 2:0:0:0 is attached to ata port 3(if there are USB disks, things may change). And the ata port device is reponsible for ata port level PM operations like power off

the
port etc., and its device is named as atax, where x is its port number.
So you can see from the log, only after sd 4:0:0:0 and sd 2:0:0:0 failed
in their ->suspend function, ata5 and ata3's suspend functions start to
run, and then sata_nv 0000:00:05.1 starts to run.

Hello again,

Quoted 5 lines · Aaron Lu

The device that is responsible for PM operations for the disk is named something like sd X:0:0:0, where X is the host number, and is normally attached under the ata port number+1. So sd 2:0:0:0 is attached to ata port 3(if there are USB disks, things may change). And the ata port device is reponsible for ata port level PM operations like power off

Quoted 8 lines

the
port etc., and its device is named as atax, where x is its port number.
So you can see from the log, only after sd 4:0:0:0 and sd 2:0:0:0 failed
in their ->suspend function, ata5 and ata3's suspend functions start to
run, and then sata_nv 0000:00:05.1 starts to run.

Yes, but 1 thing completes before disks are suspended:

[ 7378.237631] sata_nv 0000:00:05.2: __device_suspend: starts
[ 7378.249333] sata_nv 0000:00:05.2: __device_suspend: done

And then sd 2:0:0:0 starts to suspend.

This might be correct seen from a dependency point of view. But what if the HW designer was a bit lazy (or saved a bit of logic), and performs some sort of shutdown when the first function (0000:00:05.2) is suspended? That would require all disks attached to be dependent on all functions.

If it is the case (which would not surprise me), is it possible to describe such dependencies in the PM?

Quoted 22 lines · Peter Dons Tychsen

Hello again,

Quoted 5 lines · Aaron Lu

The device that is responsible for PM operations for the disk is named something like sd X:0:0:0, where X is the host number, and is normally attached under the ata port number+1. So sd 2:0:0:0 is attached to ata port 3(if there are USB disks, things may change). And the ata port device is reponsible for ata port level PM operations like power off

Quoted 8 lines

the
port etc., and its device is named as atax, where x is its port number.
So you can see from the log, only after sd 4:0:0:0 and sd 2:0:0:0 failed
in their ->suspend function, ata5 and ata3's suspend functions start to
run, and then sata_nv 0000:00:05.1 starts to run.

Yes, but 1 thing completes before disks are suspended:

[ 7378.237631] sata_nv 0000:00:05.2: __device_suspend: starts
[ 7378.249333] sata_nv 0000:00:05.2: __device_suspend: done

Quoted 7 lines

And then sd 2:0:0:0 starts to suspend.

This might be correct seen from a dependency point of view. But what if the HW designer was a bit lazy (or saved a bit of logic), and performs some sort of shutdown when the first function (0000:00:05.2) is suspended? That would require all disks attached to be dependent on all functions.

I am not sure that inter-function dependencies are the issue here (although given these NVIDIA controllers, anything is possible). Looking at the dmesg output posted on the Bugzilla report, it looks like although bladud's machine has multiple controller instances, Joe Sapp's only has one, so that can't be the problem there.

I can't tell about bladud's machine but Joe Sapp's is using SWNCQ

mode. Looking at the nv_swncq_port_suspend and nv_swncq_port_resume functions in sata_nv, they seem suspicious. They seem to act more globally than the port they are called for, doing things like disabling interrupts and SWNCQ mode for all ports on that host. It appears that if any of the ports on a host was suspended, all of them would be disabled.

It looks like it would be fixable, though since there's no proper public docs on this chipset it would be done kind of blindly.

Quoted 1 line

I can't tell about bladud's machine but Joe Sapp's is using SWNCQ

Well, we definitely need bladud to give us the *full* dmesg :-)

Quoted 3 lines

Quoted 1 line

I can't tell about bladud's machine but Joe Sapp's is using SWNCQ

Well, we definitely need bladud to give us the *full* dmesg :-)

Ok, sorry. Full dmesg with debug info and successful suspend attached to bug report.

Simeon

And also full dmesg for the failed case please, thanks.

Quoted 1 line

And also full dmesg for the failed case please, thanks.

Done (but it is the same boot, so includes the successful case as a subset)