On Dec 1, 2020, at 10:44 AM, Eric Sandeen [off-list ref] wrote:

On 12/1/20 11:32 AM, Darrick J. Wong wrote:

quoted

On Tue, Dec 01, 2020 at 10:57:11AM -0600, Eric Sandeen wrote:

quoted

STATX_ATTR_MOUNT_ROOT and STATX_ATTR_DAX got merged with the same value,
so one of them needs fixing. Move STATX_ATTR_DAX.

While we're in here, clarify the value-matching scheme for some of the
attributes, and explain why the value for DAX does not match.

Signed-off-by: Eric Sandeen <redacted>
---
include/uapi/linux/stat.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
index 82cc58fe9368..9ad19eb9bbbf 100644
--- a/include/uapi/linux/stat.h
+++ b/include/uapi/linux/stat.h

@@ -171,9 +171,10 @@ struct statx {
 * be of use to ordinary userspace programs such as GUIs or ls rather than
 * specialised tools.
 *
- * Note that the flags marked [I] correspond to generic FS_IOC_FLAGS
+ * Note that the flags marked [I] correspond to the FS_IOC_SETFLAGS flags
 * semantically.  Where possible, the numerical value is picked to correspond
- * also.
+ * also. Note that the DAX attribute indicates that the inode is currently
+ * DAX-enabled, not simply that the per-inode flag has been set.

I don't really like using "DAX-enabled" to define "DAX attribute".  How
about cribbing from the statx manpage?

"Note that the DAX attribute indicates that the file is in the CPU
direct access state.  It does not correspond to the per-inode flag that
some filesystems support."

Sure.  Consistency and specificity is good, I'll change that.

quoted

quoted

 */
#define STATX_ATTR_COMPRESSED		0x00000004 /* [I] File is compressed by the fs */
#define STATX_ATTR_IMMUTABLE		0x00000010 /* [I] File is marked immutable */

@@ -183,7 +184,7 @@ struct statx {

#define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
#define STATX_ATTR_MOUNT_ROOT		0x00002000 /* Root of a mount */
#define STATX_ATTR_VERITY		0x00100000 /* [I] Verity protected file */
-#define STATX_ATTR_DAX			0x00002000 /* [I] File is DAX */
+#define STATX_ATTR_DAX			0x00400000 /* File is currently DAX-enabled */

Why not use the next bit in the series (0x200000)?  Did someone already
claim it in for-next?

Since it didn't match the FS_IOC_SETFLAGS flag, I was trying to pick one that
seemed unlikely to ever gain a corresponding statx flag, and since 0x00400000 is
"reserved for ext4" it seemed like a decent choice.

But 0x200000 corresponds to FS_EA_INODE_FL/EXT4_EA_INODE_FL which is ext4-specific
as well, so sure, I'll change to that.

If you look a few lines up in the context, this is supposed to be using the
same value as the other inode flags:

 * Note that the flags marked [I] correspond to generic FS_IOC_FLAGS
 * semantically.  Where possible, the numerical value is picked to correspond
 * also.

#define FS_DAX_FL                       0x02000000 /* Inode is DAX */
#define EXT4_DAX_FL                     0x02000000 /* Inode is DAX */

(FS_DAX_FL also used by XFS) so this should really be "0x02000000" instead
of some other value.

Cheers, Andreas