On 2022-07-13 01:17, Christoph Hellwig wrote: > On Tue, Jul 12, 2022 at 05:13:48PM -0600, Logan Gunthorpe wrote: >>> + >>> + percpu_ref_exit(&mddev->writes_pending); >>> + bioset_exit(&mddev->bio_set); >>> + bioset_exit(&mddev->sync_set); >>> + >>> + kfree(mddev); >>> +} >> >> I still don't think this is entirely correct. There are error paths that >> will put the kobject before the disk is created and if they get hit then >> the kfree(mddev) will never be called and the memory will be leaked. > > True. > >> Instead of creating an ugly special path for that, I came up with a solution >> that I think makes a bit more sense: the kobject is still freed in it's >> own free function, but the disk holds a reference to the kobject and drops >> it in its free function. The sysfs puts and del_gendisk are then moved >> into mddev_delayed_delete() so they happen earlier. > > I'm not sure this is a good idea. The mddev kobject hangs off the > disk, so I don't think that it should in any way control the life > time of the disk, as that just creates potential problems down the > road. My interpretation was that kobject_del() (which is called in mddev_delayed_delete() before the disk would be removed) removes the mddev from the disk. At that point, it's just a structure hanging around that will be freed when its last reference is dropped, which is the disk itself cleaning up. I'm not sure how that would cause potential problems. Logan
