Re: [PATCH v8 09/10] block, scsi: Make SCSI quiesce and resume work reliably

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux