On Thu, Mar 21, 2019 at 08:17:44PM -0700, Bart Van Assche wrote:
> On 3/21/19 7:11 PM, Ming Lei wrote:
> > On Fri, Mar 22, 2019 at 09:45:54AM +0800, zhengbin wrote:
> > > + /* If device state changes to SDEV_RUNNING, we need to run hw queue
> > > +  * to avoid io hung.
> > > +  */
> > > + if (ret == 0 && state == SDEV_RUNNING)
> > > +         blk_mq_run_hw_queues(sdev->request_queue, true);
> > > 
> > 
> > Thinking of further, this way isn't safe, given the caller should make
> > sure that the queue is alive before calling blk_mq_run_hw_queues().
> > 
> > Now it is in sysfs write path, and the scsi device can become gone
> > just before calling blk_mq_run_hw_queues().
> 
> Hi Ming,
> 
> What makes you think that this approach is unsafe? I think that
> __scsi_remove_device() waits until all sysfs callbacks have finished before
> it calls blk_cleanup_queue().

You mean sysfs_remove_groups() will wait for completion of the current
store?

If sysfs would provide such guarantee, it is safe.

Looks too tricky even though it is true, so I still suggest to run
queue with holding the mutex of state_mutex, given this usage has been done
by __scsi_remove_device() already.

Thanks,
Ming

Reply via email to