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

Kent Overstreet
2024-10-16T09:49:33Z

On Wed, Oct 16, 2024 at 09:00:42AM +0200, Greg Kroah-Hartman wrote:
> [meta comment, Kent, I'm not getting your emails sent to me at all, they
> aren't even showing up in the gmail spam box, so something is really off
> on your server such that google is just rejecting them all?]

I'm just using Migadu, if it persists it'll have to be escalated with
both Google and Migadu to get anything done, most likely.

On Wed, Oct 16, 2024 at 09:00:42AM +0200, Greg Kroah-Hartman wrote:
> > There was already past precedent with the block/holder.c code, and
> > userspace does depend on that for determining the topology of virtual
> > block devices.
>
> What tools use that? What sysfs links are being created for it?
>
> And yes, filesystems do poke around in sysfs, but they almost always do
> so in a racy way, see this old link for examples of common problems:
> https://lore.kernel.org/all/20230406120716.80980-1-frank.li@vivo.com/#r

That doesn't appear to be at all relevant to this discussion. Most/all
of the major filesystems today do have objects in sysfs under /sys/fs,
which is what that thread was describing, and I know some of those
people are going to take issue if you're calling their code buggy.

> > And that really is what sysfs is for, determining the actual topology
> > and relationships between various devices - so if there's a relationship
> > between devices we need to be able to expose that.
>
> I totally agree, that is what sysfs is for, but at the filesystem layer
> you all are having to deal with "raw" kobjects and doing that gets
> tricky and is easy to get wrong.

Well, you're the person who created the API.

> > Re: the safety issues, I don't agree - provided you have a stable
> > reference to the underlying kobject, which we do, since we have the
> > block device open. The race is only w.r.t. kobj->state_in_sysfs, and
> > that could be handled easily within the sysfs/kobject code.
>
> Handled how?

Per-kobject lock, taken by kobject_add() and kobject_del(), to
synchronize kobj->state_in_sysfs and the actual VFS state;
sysfs_create_link() and sysfs_remove_link() takes the same lock. It's
not hard...

> > The alternative would be scanning through /proc/mounts, which is really
> > nasty - the format isn't particularly cleanly specified, it's racy, and
> > with containers systems are getting into the thousands of mounts these
> > days.
>
> How does all other filesystems do this? Surely we are not relying on
> each filesystem to create these symlinks, that's just not going to
> work...

Sysfs code is currently in no way standardized across filesystems. I
recently introduced standard vfs-layer ioctls for getting the UUID and
sysfs paths of mounted filesystems, but we're a long ways from any real
standardization.