Re: [PATCH V2 09/13] scsi: force unfreezing queue into atomic mode

[Ming Lei <ming.lei@redhat.com>]

On Tue, Jan 25, 2022 at 08:27:39AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 25, 2022 at 07:21:55AM +0800, Ming Lei wrote:
> > > > @@ -3670,7 +3670,7 @@ static void scsi_disk_release(struct device *dev)
> > > >  	 * in case multiple processes open a /dev/sd... node concurrently.
> > > >  	 */
> > > >  	blk_mq_freeze_queue(q);
> > > > -	blk_mq_unfreeze_queue(q);
> > > > +	__blk_mq_unfreeze_queue(q, true);
> > > 
> > > I think the right thing here is to drop the freeze/unfreeze pair.
> > > Now that del_gendisk properly freezes the queue, we don't need this
> > > protection as the issue that Bart fixed with it can't happen any more.
> > 
> > As you see, the last patch removes freeze/unfreeze pair in del_gendisk(),
> > which looks not very useful: it can't drain IO on bio based driver, and
> > del_gendisk() is supposed to provide consistent behavior for both request
> > and bio based driver.
> 
> So what is the advantage of trying to remove the freeze from where
> it belongs (common unregister code) while keeping it where it is a bandaid
> (driver specific unregister code)?

freeze in common unregister code is actually not good, because it provide
nothing for bio based driver, so we can't move blk-cgroup shutdown into
del_gendisk. Also we can't move elevator shutdown to del_gendisk for
similar reason.

Secondly freeze is pretty slow in percpu mode, so why slow down removing every
disk just for scsi's bandaid?


Thanks,
Ming