Re: REGRESSION: [PATCH 4/4] block: freeze the queue earlier in del_gendisk

[Christoph Hellwig <hch@lst.de>]

On Sun, Jul 10, 2022 at 09:33:51PM -0600, Logan Gunthorpe wrote:
> I did a fairly quick review of the patches:
> 
>  - In the patch that introduces md_free_disk() it looks like md_free()
> can still be called from the error path of md_alloc() before
> mddev->gendisk is set... which seems to make things rather complicated
> seeing we then can't use free_disk to finish the cleanup if the disk
> hasn't been created yet. I probably need to take closer look at this
> but, it might make more sense for the cleanup to remain in md_free() but
> call kobject_put() in md_free_disk() and del_gendisk() in
> mdev_delayed_delete(). Then md_alloc() can still use kobject_put() in
> the error path and it makes a little more sense seeing we'd still be
> freeing the kobject stuff in it's own release method instead of the
> disks free method.

Uww, yes.  I suspect the best fix is to actually stop the kobject
from taking part in the md life time directly.  Because the kobject
contributes a reference to the disk until it is deleted, we might
as well stop messing with the refcounts entirely and just call
kobject_del on it just before del_gendisk.  Let me see what I can do
there.

> 
>  - In the patch with md_rdevs_overlap, it looks like we remove the
> rcu_read_lock(), which definitely seems out of place and probably isn't
> correct. But the comment that was recreated still references the rcu so
> probably should be changed.

Fixed.

>  - The last patch has a typo in the title (disk is *a* freed).

Fixed.