Re: [PATCH] xfs/XXX: Add xfs/XXX

[Amir Goldstein]

On Mon, Apr 13, 2020 at 9:06 AM <ira.weiny@xxxxxxxxx> wrote:
>
> From: Ira Weiny <ira.weiny@xxxxxxxxx>
>
> Add XXX to test per file DAX operations.

Please change commit title to "xfs: Add test for per file DAX operations"
The title Add xfs/XXX is not useful even if XXX where a number.

But the kernel patch suggests that there is an intention to make
this behavior also applicable to ext4??
If that is the case I would recommend making this a generic tests
which requires filesystem support for -o dax=XXX

>
> The following is tested[*]
>
>  1. There exists an in-kernel access mode flag S_DAX that is set when
>     file accesses go directly to persistent memory, bypassing the page
>     cache.  Applications must call statx to discover the current S_DAX
>     state (STATX_ATTR_DAX).
>
>  2. There exists an advisory file inode flag FS_XFLAG_DAX that is
>     inherited from the parent directory FS_XFLAG_DAX inode flag at file
>     creation time.  This advisory flag can be set or cleared at any
>     time, but doing so does not immediately affect the S_DAX state.
>
>     Unless overridden by mount options (see (3)), if FS_XFLAG_DAX is set
>     and the fs is on pmem then it will enable S_DAX at inode load time;
>     if FS_XFLAG_DAX is not set, it will not enable S_DAX.
>
>  3. There exists a dax= mount option.
>
>     "-o dax=never"  means "never set S_DAX, ignore FS_XFLAG_DAX."
>
>     "-o dax=always" means "always set S_DAX (at least on pmem),
>                     and ignore FS_XFLAG_DAX."
>
>     "-o dax"        is an alias for "dax=always".
>
>     "-o dax=inode"  means "follow FS_XFLAG_DAX" and is the default.
>
>  4. There exists an advisory directory inode flag FS_XFLAG_DAX that can
>     be set or cleared at any time.  The flag state is copied into any
>     files or subdirectories when they are created within that directory.
>
>  5. Programs that require a specific file access mode (DAX or not DAX)
>     can do one of the following:
>
>     (a) Create files in directories that the FS_XFLAG_DAX flag set as
>         needed; or
>
>     (b) Have the administrator set an override via mount option; or
>
>     (c) Set or clear the file's FS_XFLAG_DAX flag as needed.  Programs
>         must then cause the kernel to evict the inode from memory.  This
>         can be done by:
>
>         i>  Closing the file and re-opening the file and using statx to
>             see if the fs has changed the S_DAX flag; and
>
>         ii> If the file still does not have the desired S_DAX access
>             mode, either unmount and remount the filesystem, or close
>             the file and use drop_caches.
>
>  6. It's not unreasonable that users who want to squeeze every last bit
>     of performance out of the particular rough and tumble bits of their
>     storage also be exposed to the difficulties of what happens when the
>     operating system can't totally virtualize those hardware
>     capabilities.  Your high performance sports car is not a Toyota
>     minivan, as it were.
>
> [*] 20200409165927.GD6741@magnolia/">https://lore.kernel.org/lkml/20200409165927.GD6741@magnolia/
>
> Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
>
> ---
> Changes from v6 (kernel patch set):
>         Start versions tracking the kernel patch set.
>         Update for new requirements
>
> Changes from V1 (xfstests patch):
>         Add test to ensure moved files preserve their flag
>         Check chattr of non-dax flags (check bug found by Darrick)
> ---
...
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -511,3 +511,4 @@
>  511 auto quick quota
>  512 auto quick acl attr
>  513 auto mount
> +999 auto

The test looks also 'quick'

Thanks,
Amir.