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.
