On Fri, Feb 16, 2024 at 07:40:34AM -0500, Brian Foster wrote: > On Fri, Feb 16, 2024 at 09:00:17AM +0100, Christoph Hellwig wrote: > > On Thu, Feb 15, 2024 at 07:24:23PM -0500, Kent Overstreet wrote: > > > > It looks like the warning could be avoided in bcachefs by checking for > > > > whether the parent dir/node still exists at cleanup time, but I'm not > > > > familiar enough with kobj management to say whether that's the > > > > right/best solution. It also looks a little odd to me to see a > > > > /sys/block/<dev>/bcachefs dir when I've not seen any other fs or driver > > > > do such a thing in the block sysfs dir(s). > > > > > > > > Any thoughts on this from the block subsystem folks? Is it reasonable to > > > > leave this link around and just fix the removal check, or is another > > > > behavior preferred? Thanks. > > > > This is the general problem with random cross-subsystem sysfs reference, > > and why they are best avoided. The block layer tears down all the sysfs > > objects at del_gendisk time as no one should start using the sysfs files > > at that point, but a mounted file system or other opener will of course > > keep the bdev itself alive. > > > > Yeah, makes sense. The fact that the dir goes away despite having the > bdev open is partly what made this seem a little odd to me. > > > I'm not sure why bcachefs is doing this, but no one really should be > > using the block layer sysfs structures and pointers except for the block > > layer itself. > > > > From Kent's comments it sounds like it was just some loose carryover > from old bcache stuff. I had poked around a bit for anything similar and > it looked to me that current bcache doesn't do this either, but I could > have missed something. > > > > so there's an existing bd_holder mechanism that e.g. device mapper uses > > > for links between block devices. I think the "this block device is going > > > away" code knows how to clean those up. > > > > > > We're not using that mechanism - and perhaps we should have been, I'd > > > need a time machine to ask myself why I did it that way 15 years back. > > > > Well, at least Tejun had a very strong opinion that no one should be > > abusing sysfs symlinks for linking up subsystems at all, see commit > > 49731baa41df404c2c3f44555869ab387363af43, which is also why this code > > is marked deprecated and we've not added additional users. > > > > Thanks. I'll send a patch to remove this once I'm back from vacation. No, we can't remove it - userspace needs to know this topology. When we've got one sysfs node with a direct relationship to another sysfs node, that needs to be reflected in syfs. What I'm hoping for is that we can get filesystems in general to do this right, not just bcachefs, so that they finally start showing up in the tree lsblk reports.
