Re: [PATCH V6 3/8] fs/stat: Define DAX statx attribute

Dave Chinner
2020-04-07T23:47:58Z

On Tue, Apr 07, 2020 at 11:29:53AM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> In order for users to determine if a file is currently operating in DAX
> state (effective DAX). Define a statx attribute value and set that
> attribute if the effective DAX flag is set.
>
> To go along with this we propose the following addition to the statx man
> page:
>
> STATX_ATTR_DAX
>
> The file is in the DAX (cpu direct access) state. DAX state
> attempts to minimize software cache effects for both I/O and
> memory mappings of this file. It requires a file system which
> has been configured to support DAX.
>
> DAX generally assumes all accesses are via cpu load / store
> instructions which can minimize overhead for small accesses, but
> may adversely affect cpu utilization for large transfers.
>
> File I/O is done directly to/from user-space buffers and memory
> mapped I/O may be performed with direct memory mappings that
> bypass kernel page cache.
>
> While the DAX property tends to result in data being transferred
> synchronously, it does not give the same guarantees of O_SYNC
> where data and the necessary metadata are transferred together.
>
> A DAX file may support being mapped with the MAP_SYNC flag,
> which enables a program to use CPU cache flush instructions to
> persist CPU store operations without an explicit fsync(2). See
> mmap(2) for more information.
>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
> ---
> Changes from V2:
> Update man page text with comments from Darrick, Jan, Dan, and
> Dave.
> ---
> fs/stat.c | 3 +++
> include/uapi/linux/stat.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/fs/stat.c b/fs/stat.c
> index 030008796479..894699c74dde 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -79,6 +79,9 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat,
> if (IS_AUTOMOUNT(inode))
> stat->attributes |= STATX_ATTR_AUTOMOUNT;
>
> + if (IS_DAX(inode))
> + stat->attributes |= STATX_ATTR_DAX;
> +
> if (inode->i_op->getattr)
> return inode->i_op->getattr(path, stat, request_mask,
> query_flags);
> diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
> index ad80a5c885d5..e5f9d5517f6b 100644
> --- a/include/uapi/linux/stat.h
> +++ b/include/uapi/linux/stat.h
> @@ -169,6 +169,7 @@ struct statx {
> #define STATX_ATTR_ENCRYPTED 0x00000800 /* [I] File requires key to decrypt in fs */
> #define STATX_ATTR_AUTOMOUNT 0x00001000 /* Dir: Automount trigger */
> #define STATX_ATTR_VERITY 0x00100000 /* [I] Verity protected file */
> +#define STATX_ATTR_DAX 0x00002000 /* [I] File is DAX */
>
>
> #endif /* _UAPI_LINUX_STAT_H */

Looks fine. Man page text seems ok, too.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com