by Steven Whitehouse

[permalink] [raw]

Subject: Re: [PATCH 00/17] VFS: Filesystem information and notifications [ver #17]

Hi,

On 25/02/2020 15:28, James Bottomley wrote:
> On Tue, 2020-02-25 at 12:13 +0000, Steven Whitehouse wrote:
>> Hi,
>>
>> On 24/02/2020 15:28, Miklos Szeredi wrote:
>>> On Mon, Feb 24, 2020 at 3:55 PM James Bottomley
>>> <[email protected]> wrote:
>>>
>>>> Once it's table driven, certainly a sysfs directory becomes
>>>> possible. The problem with ST_DEV is filesystems like btrfs and
>>>> xfs that may have multiple devices.
>>> For XFS there's always a single sb->s_dev though, that's what
>>> st_dev will be set to on all files.
>>>
>>> Btrfs subvolume is sort of a lightweight superblock, so basically
>>> all such st_dev's are aliases of the same master superblock. So
>>> lookup of all subvolume st_dev's could result in referencing the
>>> same underlying struct super_block (just like /proc/$PID will
>>> reference the same underlying task group regardless of which of the
>>> task group member's PID is used).
>>>
>>> Having this info in sysfs would spare us a number of issues that a
>>> set of new syscalls would bring. The question is, would that be
>>> enough, or is there a reason that sysfs can't be used to present
>>> the various filesystem related information that fsinfo is supposed
>>> to present?
>>>
>>> Thanks,
>>> Miklos
>>>
>> We need a unique id for superblocks anyway. I had wondered about
>> using s_dev some time back, but for the reasons mentioned earlier in
>> this thread I think it might just land up being confusing and
>> difficult to manage. While fake s_devs are created for sbs that don't
>> have a device, I can't help thinking that something closer to
>> ifindex, but for superblocks, is needed here. That would avoid the
>> issue of which device number to use.
>>
>> In fact we need that anyway for the notifications, since without
>> that there is a race that can lead to missing remounts of the same
>> device, in case a umount/mount pair is missed due to an overrun, and
>> then fsinfo returns the same device as before, with potentially the
>> same mount options too. So I think a unique id for a superblock is a
>> generically useful feature, which would also allow for sensible sysfs
>> directory naming, if required,
> But would this be informative and useful for the user? I'm sure we can
> find a persistent id for a persistent superblock, but what about tmpfs
> ... that's going to have to change with every reboot. It's going to be
> remarkably inconvenient if I want to get fsinfo on /run to have to keep
> finding what the id is.

That is a different question though, or at least it might be... the idea
of the superblock id is to uniquely identify a particular superblock.
The mount notification should give you the association between that
superblock and any devices (assuming those are applicable), or you can
use fsinfo if you were not listening to the notifications at the time of
the mount to get the same information.

If someone unmounts /run and remounts it, then the superblock id would
change, but otherwise it would stay the same, so you know that it is the
same mount that is being described in future notifications. One of the
main aims here being to combine the fsinfo information with the
notifications in a race free manner.

There are a number of ways one might want to specify a filesystem: by
device, by uuid, by volume label and so forth but we can't use any of
those very easily as a unique id. Someone might remove a drive and
replace it with a different one (so same device, but different content)
or they might have two filesystems with the same uuid if they've just
done a dd copy to a new device. For the mount notifications we need
something that doesn't suffer from these issues, but which can also be
very easily associated with what in most cases are more convenient ways
to specify a particular filesystem.

>
> The other thing a file descriptor does that sysfs doesn't is that it
> solves the information leak: if I'm in a mount namespace that has no
> access to certain mounts, I can't fspick them and thus I can't see the
> information. By default, with sysfs I can.
>
> James
>
Yes, thats true, and I wasn't advocating for the sysfs method over
fspick here, just pointing out that a unique superblock id would be a
generically useful thing to have,

Steve.

2020-02-26 02:29:55

2020-03-03 21:51:57

On Sat, Mar 7, 2020 at 10:48 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Fri, Mar 06, 2020 at 05:25:49PM +0100, Miklos Szeredi wrote:
> > On Tue, Mar 03, 2020 at 08:46:09AM +0100, Miklos Szeredi wrote:
> > >
> > > I'm doing a patch. Let's see how it fares in the face of all these
> > > preconceptions.
> >
> > Here's a first cut. Doesn't yet have superblock info, just mount info.
> > Probably has rough edges, but appears to work.
> >
> > I started with sysfs, then kernfs, then went with a custom filesystem, because
> > neither could do what I wanted.
>
> Hm, what is wrong with kernfs that prevented you from using it here?
> Just complexity or something else?

I wanted to have a single instance covering all the namespaces, with
just a filtered view depending on which namespace the task is looking
at it.

Having a kernfs_node for each attribute is also rather heavy compared
to the size of struct mount.

Thanks,
Miklos