Re: [PATCH] bcachefs: Fix sysfs warning in fstests generic/730,731

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Oct 14, 2024 at 08:10:19AM +0200, Christoph Hellwig wrote:
> On Sat, Oct 12, 2024 at 02:42:39PM -0400, Kent Overstreet wrote:
> > sysfs warns if we're removing a symlink from a directory that's no
> > longer in sysfs; this is triggered by fstests generic/730, which
> > simulates hot removal of a block device.
> > 
> > This patch is however not a correct fix, since checking
> > kobj->state_in_sysfs on a kobj owned by another subsystem is racy.
> > 
> > A better fix would be to add the appropriate check to
> > sysfs_remove_link() - and sysfs_create_link() as well.
> 
> The proper fix is to not link to random other subsystems with
> object lifetimes you can't know.  I'm not sure why you think adding
> this link was ever allowed.
> 

Odd, I never got the original patch that was sent here in the first
place...

Anyway, Christoph is right, this patch isn't ok.  You can't link outside
of the subdirectory in which you control in sysfs without a whole lot of
special cases and control.  The use of sysfs for filesystems is almost
always broken and tricky and full of race conditions (see many past
threads about this.)  Ideally we would fix this up by offering common
code for filesystems to use for sysfs (like we do for the driver
subsystems), but no one has gotten around to it for various reasons.

The only filesystem that I can see that attempts to do much like what
bcachefs does in sysfs is btrfs, but btrfs only seems to have one
symlink, while you have multiple ones pointing to the same block device.

I can't find any sysfs documentation in Documentation/ABI/ so I don't
really understand what it's attempting to do (and why isn't the tools
that check this screaming about that lack of documentation, that's
odd...)  Any hints as to what you are wishing to show here?

thanks,

greg k-h