On 05/21/2014 09:21 PM, Jeff Mahoney wrote: > On 05/21/2014 08:12 PM, Chris Mason wrote: >> >> The Btrfs sysfs code removes entries for raid types that are no >> longer in use. This means that if you have a raid0 FS and use balance >> to turn it into a raid1 FS, the raid0 sysfs entries will go away. >> >> The rough chain of events is: >> >> __link_block_group() -> see we're the first RAIDX, add sysfs entry >> >> btrfs_remove_block_group() -> notice we're removing the last RAIDX >> remove sysfs entry >> >> This all makes sense until we try to add RAIDX back into the FS again. >> The problem is that our RAID kobjects are just in an array that gets >> freed at unmount time, instead of an array of pointers to kobjects that >> get freed when the great sysfs in the sky is done with them. >> >> When we remove the sysfs entry for a given raid level, the syfs code >> free's the name. When we use the same kobject to add back the RAIDX >> entry again, sysfs sees the old name pointer and tries to free it again. >> >> All of which is a long way of saying we're using sysfs wrong. For now, >> just don't remove entries for raid levels that we're no longer using. > > Hi Chris - > > Thanks for posting the problem. I disagree that sysfs is being used > wrong here - or well, not on purpose. The problem is only that I didn't > anticipate raid levels going away and only initialize the kobjects i > update_space_info.
Thanks Jeff, this is more a comment on the sadness that comes from using sysfs, not a knock on your patch ;) > > The name being double-freed is only part of the problem. The kobject > isn't reinitialized at all. Clearing ->name and re-calling kobject_init > would fix it - but I think we can do one better. > > kobject_cleanup's comment for freeing ->name claims that ->name being > set means that it was allocated by the kobject infrastructure. The raid > names come directly out of an array anyway, so we don't even need to > copy it. We can avoid the string allocation entirely and handle the > reuse properly. > > The following (untested, tonight anyway) patch should fix it. Can we safely reinit a kobject that has been put in use in sysfs? Given all the things that can hold refs etc is this legal? -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
