On Mon, 9 May 2022 at 14:48, Christian Brauner [off-list ref] wrote:

One comment about this. We really need to have this interface support
giving us mount options like "relatime" back in numeric form (I assume
this will be possible.). It is royally annoying having to maintain a
mapping table in userspace just to do:

relatime -> MS_RELATIME/MOUNT_ATTR_RELATIME
ro       -> MS_RDONLY/MOUNT_ATTR_RDONLY

A library shouldn't be required to use this interface. Conservative
low-level software that keeps its shared library dependencies minimal
will need to be able to use that interface without having to go to an
external library that transforms text-based output to binary form (Which
I'm very sure will need to happen if we go with a text-based
interface.).

Agreed.

  This pattern of requesting the size first by passing empty arguments,
  then allocating the buffer and then passing down that buffer to
  retrieve that value is really annoying to use and error prone (I do
  of course understand why it exists.).

  For real xattrs it's not that bad because we can assume that these
  values don't change often and so the race window between
  getxattr(GET_SIZE) and getxattr(GET_VALUES) often doesn't matter. But
  fwiw, the post > pre check doesn't exist for no reason; we do indeed
  hit that race.

That code is wrong.  Changing xattr size is explicitly documented in
the man page as a non-error condition:

       If size is specified as zero, these calls return the  current  size  of
       the  named extended attribute (and leave value unchanged).  This can be
       used to determine the size of the buffer that should be supplied  in  a
       subsequent  call.   (But, bear in mind that there is a possibility that
       the attribute value may change between the two calls,  so  that  it  is
       still necessary to check the return status from the second call.)

  In addition, it is costly having to call getxattr() twice. Again, for
  retrieving xattrs it often doesn't matter because it's not a super
  common operation but for mount and other info it might matter.

You don't *have* to retrieve the size, it's perfectly valid to e.g.
start with a fixed buffer size and double the size until the result
fits.

* Would it be possible to support binary output with this interface?
  I really think users would love to have an interfact where they can
  get a struct with binary info back.

I think that's bad taste.   fsinfo(2) had the same issue.  As well as
mount(2) which still interprets the last argument as a binary blob in
certain cases (nfs is one I know of).

  Especially for some information at least. I'd really love to have a
  way go get a struct mount_info or whatever back that gives me all the
  details about a mount encompassed in a single struct.

If we want that, then can do a new syscall with that specific struct
as an argument.

  Callers like systemd will have to parse text and will end up
  converting everything from text into binary anyway; especially for
  mount information. So giving them an option for this out of the box
  would be quite good.

What exactly are the attributes that systemd requires?

  Interfaces like statx aim to be as fast as possible because we exptect
  them to be called quite often. Retrieving mount info is quite costly
  and is done quite often as well. Maybe not for all software but for a
  lot of low-level software. Especially when starting services in
  systemd a lot of mount parsing happens similar when starting
  containers in runtimes.

Was there ever a test patch for systemd using fsinfo(2)?  I think not.

Until systemd people start to reengineer the mount handing to allow
for retrieving a single mount instead of the complete mount table we
will never know where the performance bottleneck lies.

* If we decide to go forward with this interface - and I think I
  mentioned this in the lsfmm session - could we please at least add a
  new system call? It really feels wrong to retrieve mount and other
  information through the xattr interfaces. They aren't really xattrs.

I'd argue with that statement.  These are most definitely attributes.
As for being extended, we'd just extended the xattr interface...

Naming aside... imagine that read(2) has always been used to retrieve
disk data, would you say that reading data from proc feels wrong?
And in hindsight, would a new syscall for the purpose make any sense?

Thanks,
Miklos