Re: [PATCH] ublk_drv: fix request queue leak

Ming Lei
2022-07-14T13:20:42Z

On Thu, Jul 14, 2022 at 06:13:41AM -0700, Christoph Hellwig wrote:
> On Thu, Jul 14, 2022 at 06:32:01PM +0800, Ming Lei wrote:
> > However, ublk may not add disk in case of starting device failure, then
> > del_gendisk() won't be called when removing ublk device, so blk_mq_exit_queue
> > will not be callsed, and it can be bit hard to deal with this kind of
> > merge conflict.
>
> So base it on a tree that has everything you need.
>
> > Turns out ublk's queue/disk use model is very similar with scsi, so switch
> > to scsi's model by allocating disk and queue independently, then it can be
> > quite easy to handle v5.20 merge conflict by replacing blk_cleanup_queue
> > with blk_mq_destroy_queue.
>
> Don't do that. That thing really is a workaround for the lack of admin
> queues in scsi. Nothing newly designed should use it. It will not
> allow to optimize things and cause maintainaince burden down the road.

The problem is that you moved part of blk_cleanup_queue() into
del_gendisk().

Here, the issue Jens reproduced is that we don't add disk yet, so won't
call del_gendisk(). The queue & disk is allocated & initialized correctly.

Then how to do the part done by original blk_cleanup_queue() without calling
blk_mq_destroy_queue()?

Thanks,
Ming