Re: [PATCH 32/32] [RFC] fsinfo: Add a system call to allow querying of filesystem information [ver #8]

[Arnd Bergmann <arnd@arndb.de>]

On Mon, Jun 4, 2018 at 9:03 PM, David Howells <dhowells@redhat.com> wrote:
> Arnd Bergmann <arnd@arndb.de> wrote:
> The fsinfo() syscall truncates the reply buffer to the size the user requested
> if the user requested a smaller amount.  Take the fsinfo_supports struct for
> example:
>
>         struct fsinfo_supports {
>                 __u64   supported_stx_attributes;
>                 __u32   supported_stx_mask;
>                 __u32   supported_ioc_flags;
>         };
>
> Now imagine that in future we want to add another field, say the mask of the
> windows file attributes a filesystem supports.  We can extend the struct like
> so:
>
>         struct fsinfo_supports_v2 {
>                 __u64   supported_stx_attributes;
>                 __u32   supported_stx_mask;
>                 __u32   supported_ioc_flags;
>                 __u32   supported_win_file_atts;
>                 __u32   __reserved[1];
>         };

Looking at the code again, I realized my misunderstanding: I somehow
expected the system call to return multiple structures at once, which
would get really messy with groups of arrays of variable-sized
structures. Since we only really get back a single structure per call,
that is not an issue.

There is also no need to be concerned about the system call overhead,
right? Even if we query all data from all mounted file systems, I
suppose the total number of syscall roundtrips will be small enough
that there is no need for complicating the interface to make it
slightly faster.

> I can improve this such that if you asked for a fixed-length option and the
> kernel doesn't have enough data to fill the user buffer provided, then it
> clears the remainder of the buffer.  That way at least any unsupported fields
> will be initialised to 0.

Yes, I think that would make sense here. It's not quite a read() based
interface since the return value for a short read is still the size of the
whole buffer that could have been accessed. By zeroing the extra
data, the kernel always writes the amount of data that the user asks
for, and the return value always shows how much data would have
been available.

It might be necessary to limit the size of the buffer though, to prevent
bad things from happening when the user asks for e.g. -1ull bytes
of data.

>> In any case, it would be nice to have a trivial way to query which of
>> the four timestamp types are supported at all, and returning
>> them separately would be one way of doing that.
>
>         fsinfo_cap_has_atime    = 45,   /* fs supports access time */
>         fsinfo_cap_has_btime    = 46,   /* fs supports birth/creation time */
>         fsinfo_cap_has_ctime    = 47,   /* fs supports change time */
>         fsinfo_cap_has_mtime    = 48,   /* fs supports modification time */

Ok.

      Arnd