lkml.org 
[lkml]   [2018]   [Jun]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 32/32] [RFC] fsinfo: Add a system call to allow querying of filesystem information [ver #8]
    On Fri, May 25, 2018 at 2:08 AM, David Howells <dhowells@redhat.com> wrote:

    > +
    > +static int fsinfo_generic_timestamp_info(struct dentry *dentry,
    > + struct fsinfo_timestamp_info *ts)
    > +{
    > + struct super_block *sb = dentry->d_sb;
    > +
    > + /* If unset, assume 1s granularity */
    > + u16 mantissa = 1;
    > + s8 exponent = 0;
    > +
    > + ts->minimum_timestamp = S64_MIN;
    > + ts->maximum_timestamp = S64_MAX;
    > + if (sb->s_time_gran < 1000000000) {
    > + if (sb->s_time_gran < 1000)
    > + exponent = -9;
    > + else if (sb->s_time_gran < 1000000)
    > + exponent = -6;
    > + else
    > + exponent = -3;
    > + }

    ntfs has sb->s_time_gran=100, and vfat should really have
    sb->s_time_gran=2000000000 but that doesn't seem to be set right
    at the moment.

    > +/*
    > + * Optional fsinfo() parameter structure.
    > + *
    > + * If this is not given, it is assumed that fsinfo_attr_statfs instance 0 is
    > + * desired.
    > + */
    > +struct fsinfo_params {
    > + enum fsinfo_attribute request; /* What is being asking for */
    > + __u32 Nth; /* Instance of it (some may have multiple) */
    > + __u32 at_flags; /* AT_SYMLINK_NOFOLLOW and similar flags */
    > + __u32 __spare[6]; /* Spare params; all must be 0 */
    > +};

    I fear the 'enum' in the uapi structure may have a different size depending
    on the architecture. Maybe turn that into a __u32 as well?

    > +struct fsinfo_capabilities {
    > + __u64 supported_stx_attributes; /* What statx::stx_attributes are supported */
    > + __u32 supported_stx_mask; /* What statx::stx_mask bits are supported */
    > + __u32 supported_ioc_flags; /* What FS_IOC_* flags are supported */
    > + __u8 capabilities[(fsinfo_cap__nr + 7) / 8];
    > +};

    This looks a bit odd: with the 44 capabilities, you end up having a
    six-byte array
    followed by two bytes of implicit padding. If the number of
    capabilities grows beyond
    64, you have a nine byte array with more padding to the next alignof(__u64). Is
    that intentional?

    How about making it a fixed size with either 64 or 128 capability bits?

    > +/*
    > + * Information struct for fsinfo(fsinfo_attr_timestamp_info).
    > + */
    > +struct fsinfo_timestamp_info {
    > + __s64 minimum_timestamp; /* Minimum timestamp value in seconds */
    > + __s64 maximum_timestamp; /* Maximum timestamp value in seconds */
    > + __u16 atime_gran_mantissa; /* Granularity(secs) = mant * 10^exp */
    > + __u16 btime_gran_mantissa;
    > + __u16 ctime_gran_mantissa;
    > + __u16 mtime_gran_mantissa;
    > + __s8 atime_gran_exponent;
    > + __s8 btime_gran_exponent;
    > + __s8 ctime_gran_exponent;
    > + __s8 mtime_gran_exponent;
    > +};

    This structure has a slightly inconsistent amount of padding at the end:
    on x86-32 it has no padding, everywhere else it has 32 bits of padding
    to make it 64-bit aligned. Maybe add a __u32 reserved field?

    > +
    > +#define __NR_fsinfo 326

    Hardcoding the syscall number in the example makes it architecture specific.
    Could you include <asm/unistd.h> to get the real number?

    Arnd

    \
     
     \ /
      Last update: 2018-06-04 15:10    [W:2.082 / U:0.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site