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
