On Mon, 2017-10-09 at 14:16 -0700, Bart Van Assche wrote:
> On Sat, 2017-10-07 at 12:33 +0800, Ming Lei wrote:
> > On Wed, Oct 04, 2017 at 05:01:10PM -0700, Bart Van Assche wrote:
> > > int
> > > scsi_device_quiesce(struct scsi_device *sdev)
> > > {
> > > + struct request_queue *q = sdev->request_queue;
> > > int err;
> > >
> > > + blk_mq_freeze_queue(q);
> > > + if (blk_set_preempt_only(q)) {
> > > + blk_mq_unfreeze_queue(q);
> > > + return -EINVAL;
> > > + }
> >
> > This way is wrong, if blk_set_preempt_only() returns true
> > it means the queue has been in PREEMPT_ONLY already,
> > and failing scsi_device_quiesce() can break suspend/resume or
> > sending SCSI domain validation command.
>
> That's an interesting comment but I propose that what you suggest is implemented
> through a separate patch. The above code preserves the existing behavior, namely
> that scsi_device_quiesce() returns an error code if called when a SCSI device has
> already been quiesced. See also scsi_device_set_state(). BTW, I don't think that
> it is likely that this will occur. The only code other than the power management
> code I know of that sets the SCSI QUIESCE state is the SCSI sysfs state store
> method and the SCSI parallel code. I don't know any software that sets the SCSI
> QUIESCE state through sysfs. And I'm not sure the SCSI parallel code has a
> significant number of users.
A correction: the current behavior when quiescing an already quiesced device is
that scsi_device_quiesce() returns 0 without changing any state. Anyway, I propose
to keep that behavior and not to add more complexity now.
Bart.
