Re: [PATCH 1/4] block: disable the elevator int del_gendisk

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 14, 2022 at 04:23:36PM +0800, Ming Lei wrote:
> >  	blk_sync_queue(q);
> >  	blk_flush_integrity();
> > +	blk_mq_cancel_work_sync(q);
> > +
> > +	blk_mq_quiesce_queue(q);
> 
> quiesce queue adds a bit long delay in del_gendisk, not sure if this way may
> cause regression in big machines with lots of disks.

It does.  But at least we remove a freeze in the queue teardown path.
But either way I'd really like to get things correct first before
looking into optimizations.

> 
> > +	if (q->elevator) {
> > +		mutex_lock(&q->sysfs_lock);
> > +		elevator_exit(q);
> > +		mutex_unlock(&q->sysfs_lock);
> > +	}
> > +	rq_qos_exit(q);
> > +	blk_mq_unquiesce_queue(q);
> 
> Also tearing down elevator here has to be carefully, that means any
> elevator reference has to hold rcu read lock or .q_usage_counter,
> meantime it has to be checked, otherwise use-after-free may be caused.

This is not a new pattern.  We have the same locking here as a
sysfs-induced change of the elevator to none which also clears
q->elevator under a queue that is frozen and quiesced.

But unlike that path we do fail all requests that could have been
queued in the schedule before unfreezing here at least.