On Fri, 2017-10-13 at 00:02 +0800, Ming Lei wrote: > On Tue, Oct 10, 2017 at 02:03:45PM -0700, Bart Van Assche wrote: > > [ ... ] > > + /* If the SCSI device has already been quiesced, do nothing. */ > > + if (blk_set_preempt_only(q)) > > + return 0; > > Last time, I have mentioned that this way isn't safe, especially > the SCSI domain command can be sent concurrently, also there might > be issue wrt. runtime PM vs. system suspend. > > The issue should have been avoided simply by setting the flag > and continue to freeze queue. I think the pm_autosleep_lock() / pm_autosleep_unlock() calls in the power management code prevent that the power management code can trigger concurrent scsi_device_quiesce() calls. > > + > > + blk_mq_freeze_queue(q); > > + /* > > + * Ensure that the effect of blk_set_preempt_only() will be visible > > + * for percpu_ref_tryget() callers that occur after the queue > > + * unfreeze even if the queue was already frozen before this function > > + * was called. See also https://lwn.net/Articles/573497/. > > + */ > > + synchronize_rcu(); > > No, it is a bad idea to do two synchronize_rcu() which can > be very slow, especially in big machine, this way may slow down > suspend much. scsi_device_quiesce() is only used by the SCSI power management code and by the SPI DV code. The number of users of the SCSI parallel code is small. And the SCSI power management code is used on laptops but not on big machines. Anyway, if you really care about this optimization, that's something that can be done easily as a follow-up patch. Bart.
