Re: [PATCH] Add a pair of system calls to make extended file stats available [ver #3]

2 messages, 2 authors, 2010-07-09 · open the first message on its own page

Re: [PATCH] Add a pair of system calls to make extended file stats available [ver #3]

From: Michael Kerrisk <hidden>
Date: 2010-07-08 19:44:09

Hi David,

A couple of comments below.

On Thu, Jul 1, 2010 at 1:36 AM, David Howells [off-list ref] wrote:
Add a pair of system calls to make extended file stats available, including
file creation time, inode version and data version where available through the
underlying filesystem.

[This depends on the previously posted pair of patches to (a) constify a number
 of syscall string and buffer arguments and (b) rearrange AFS's use of
 i_version and i_generation].

The following structures are defined for their use:

       struct xstat_parameters {
               unsigned long long      request_mask;
Poor name, since it's a value-result arg? Better maybe something like
"field_mask"?
       };

       struct xstat_dev {
               unsigned int            major, minor;
       };

       struct xstat_time {
               unsigned long long      tv_sec, tv_nsec;
       };

       struct xstat {
               unsigned int            st_mode;
               unsigned int            st_nlink;
               unsigned int            st_uid;
               unsigned int            st_gid;
               struct xstat_dev        st_rdev;
               struct xstat_dev        st_dev;
               struct xstat_time       st_atime;
               struct xstat_time       st_mtime;
               struct xstat_time       st_ctime;
               struct xstat_time       st_btime;
               unsigned long long      st_ino;
               unsigned long long      st_size;
               unsigned long long      st_blksize;
               unsigned long long      st_blocks;
               unsigned long long      st_gen;
               unsigned long long      st_data_version;
               unsigned long long      st_result_mask;
               unsigned long long      st_extra_results[0];
       };

where st_btime is the file creation time, st_gen is the inode generation
(i_generation), st_data_version is the data version number (i_version),
request_mask and st_result_mask are bitmasks of data desired/provided and
st_extra_results[] is where as-yet undefined fields are appended.

The defined bits in request_mask and st_result_mask are:

       XSTAT_REQUEST_MODE              Want/got st_mode
       XSTAT_REQUEST_NLINK             Want/got st_nlink
       XSTAT_REQUEST_UID               Want/got st_uid
       XSTAT_REQUEST_GID               Want/got st_gid
       XSTAT_REQUEST_RDEV              Want/got st_rdev
       XSTAT_REQUEST_ATIME             Want/got st_atime
       XSTAT_REQUEST_MTIME             Want/got st_mtime
       XSTAT_REQUEST_CTIME             Want/got st_ctime
       XSTAT_REQUEST_INO               Want/got st_ino
       XSTAT_REQUEST_SIZE              Want/got st_size
       XSTAT_REQUEST_BLOCKS            Want/got st_blocks
       XSTAT_REQUEST__BASIC_STATS      The stuff in the normal stat struct
       XSTAT_REQUEST_BTIME             Want/got st_btime
       XSTAT_REQUEST_GEN               Want/got st_gen
       XSTAT_REQUEST_DATA_VERSION      Want/got st_data_version
       XSTAT_REQUEST__EXTENDED_STATS   The stuff in the xstat struct
       XSTAT_REQUEST__ALL_STATS        The defined set of requestables

The system calls are:

       ssize_t ret = xstat(int dfd,
                           const char *filename,
                           unsigned flags,
                           const struct xstat_parameters *params,
                           struct xstat *buffer,
                           size_t buflen);

       ssize_t ret = fxstat(unsigned fd,
                            unsigned flags,
                            const struct xstat_parameters *params,
                            struct xstat *buffer,
                            size_t buflen);


The dfd, filename, flags and fd parameters indicate the file to query.  There
is no equivalent of lstat() as that can be emulated with xstat() by passing
AT_SYMLINK_NOFOLLOW in flags.

AT_FORCE_ATTR_SYNC can also be set in flags.  This will require a network
filesystem to synchronise its attributes with the server.

When the system call is executed, the request_mask bitmask is read from the
parameter block to work out what the user is requesting.  If params is NULL,
then request_mask will be assumed to be XSTAT_REQUEST__GET_ANYWAY.
There is no XSTAT_REQUEST__GET_ANYWAY, AFAICS. I guess here you meant
XSTAT_REQUEST__EXTENDED_STATS? Or?

The request_mask should be set by the caller to specify extra results that the
caller may desire.  These come in a number of classes:

 (0) dev, blksize.

    These are local data and are always available.

 (1) mode, nlinks, uid, gid, [amc]time, ino, size, blocks.

    These will be returned whether the caller asks for them or not.  The
    corresponding bits in result_mask will be set to indicate their presence.

    If the caller didn't ask for them, then they may be approximated.  For
    example, NFS won't waste any time updating them from the server, unless as
    a byproduct of updating something requested.

 (2) rdev.

    As for class (1), but this won't be returned if the file is not a blockdev
    or chardev.  The bit will be cleared if the value is not returned.

 (3) File creation time, inode generation and data version.

    These will be returned if available whether the caller asked for them or
    not.  The corresponding bits in result_mask will be set or cleared as
    appropriate to indicate their presence.

    If the caller didn't ask for them, then they may be approximated.  For
    example, NFS won't waste any time updating them from the server, unless
    as a byproduct of updating something requested.

 (4) Extra results.

    These will only be returned if the caller asked for them by setting their
    bits in request_mask.  They will be placed in the buffer after the xstat
    struct in ascending result_mask bit order.  Any bit set in request_mask
    mask will be left set in result_mask if the result is available and
    cleared otherwise.

    The pointer into the results list will be rounded up to the nearest 8-byte
    boundary after each result is written in.  The size of each extra result
    is specific to the definition for that result.

    No extra results are currently defined.

If the buffer is insufficiently big, the syscall returns the amount of space it
will need to write the complete result set and returns a partial result in the
buffer.
This case is almost certainly a user error, so why not simply return
an error (-1 and ERANGE or E2BIG)? The above approach invites
userspace errors of the form:

if (xtat(...) < 0) { /* How users often check for error */
    /* I'll handle the error */
} else {
    /* The call succeeded; I'm fine */
}

Instead, more complex error-handling is required for *every* call:

ret = xstat(..., buflen);
if (ret < 0 || ret > buflen)
    /* I'll handle the error */
} else {
    /* The call succeeded; I'm fine */
}

If you are looking for a way to inform the user about the required
buffer size, I think it would be better to take a leaf from the
getxattr(2) book: if 'buflen' is zero, then do nothing with the output
arg, but return the size that would be required.

Cheers,

Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add a pair of system calls to make extended file stats available [ver #3]

From: David Howells <dhowells@redhat.com>
Date: 2010-07-09 13:59:06

Michael Kerrisk [off-list ref] wrote:
quoted
       struct xstat_parameters {
               unsigned long long      request_mask;
Poor name, since it's a value-result arg? Better maybe something like
"field_mask"?
No.  The contents of xstat_parameters aren't changed.  request_mask is what
you're asking for, result_mask in the xstat struct is what you actually got.

result_mask may be more or less than request_mask as the filesystem isn't
obliged to supply anything you didn't ask for, and may not be able to supply
something you did ask for, and may give you stuff anyway that you didn't ask
for if it's trivial to do so.
There is no XSTAT_REQUEST__GET_ANYWAY, AFAICS. I guess here you meant
XSTAT_REQUEST__EXTENDED_STATS? Or?
Yep.  I forgot to change that in the patch description.
This case is almost certainly a user error, so why not simply return
an error (-1 and ERANGE or E2BIG)? The above approach invites
userspace errors of the form:

if (xtat(...) < 0) { /* How users often check for error */
    /* I'll handle the error */
} else {
    /* The call succeeded; I'm fine */
}
I suppose.
If you are looking for a way to inform the user about the required
buffer size, I think it would be better to take a leaf from the
getxattr(2) book: if 'buflen' is zero, then do nothing with the output
arg, but return the size that would be required.
That's reasonable.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help