On Mon, 2018-07-30 at 09:56 +-0800, jianchao.wang wrote: +AD4- static int sdev+AF8-runtime+AF8-suspend(struct device +ACo-dev) +AD4- +AHs- +AD4- const struct dev+AF8-pm+AF8-ops +ACo-pm +AD0- dev-+AD4-driver ? dev-+AD4-driver-+AD4-pm : NULL+ADs- +AD4- struct scsi+AF8-device +ACo-sdev +AD0- to+AF8-scsi+AF8-device(dev)+ADs- +AD4- int err +AD0- 0+ADs- +AD4- +AD4- err +AD0- blk+AF8-pre+AF8-runtime+AF8-suspend(sdev-+AD4-request+AF8-queue)+ADs- +AD4- if (err) +AD4- return err+ADs- +AD4- if (pm +ACYAJg- pm-+AD4-runtime+AF8-suspend) +AD4- err +AD0- pm-+AD4-runtime+AF8-suspend(dev)+ADs- +AD4- blk+AF8-post+AF8-runtime+AF8-suspend(sdev-+AD4-request+AF8-queue, err)+ADs- +AD4- +AD4- return err+ADs- +AD4- +AH0- +AD4- +AD4- If blk+AF8-pre+AF8-runtime+AF8-suspend returns -EBUSY, blk+AF8-post+AF8-runtime+AF8-suspend will not be invoked. Right, I will fix this. +AD4- +AD4- +AD4- The request+AF8-queue should be set to preempt only mode only when we confirm we could set +AD4- +AD4- +AD4- rpm+AF8-status to RPM+AF8-SUSPENDING or RPM+AF8-RESUMING. +AD4- +AD4- +AD4- +AD4- Why do you think this? +AD4- +AD4- https://marc.info/?l+AD0-linux-scsi+ACY-m+AD0-133727953625963+ACY-w+AD0-2 +AD4- +ACI- +AD4- If q-+AD4-rpm+AF8-status is RPM+AF8-SUSPENDED, they shouldn't do anything -- act as though the queue is +AD4- empty. If q-+AD4-rpm+AF8-status is RPM+AF8-SUSPENDING or RPM+AF8-RESUMING, they should hand over the request +AD4- only if it has the REQ+AF8-PM flag set. +AD4- +ACI- I think the blk+AF8-pre+AF8-runtime+AF8-suspend() callers guarantee that q-+AD4-rpm+AF8-status +AD0APQ- RPM+AF8-ACTIVE before blk+AF8-pre+AF8-runtime+AF8-suspend() is called. I will add a WARN+AF8-ON+AF8-ONCE() statement that verifies that. +AD4- In additon, if we set the preempt only here unconditionally, the normal IO will be blocked +AD4- during the blk+AF8-pre+AF8-runtime+AF8-suspend. In your patch, q+AF8-usage+AF8-counter will be switched to atomic mode, +AD4- this could cost some time. Is it really OK ? I will see what I can do about this. Bart.