Re: Clarification of statx->attributes_mask meaning?

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Nov 25, 2020 at 01:19:48PM -0600, Eric Sandeen wrote:
> The way attributes_mask is used in various filesystems seems a bit
> inconsistent.
> 
> Most filesystems set only the bits for features that are possible to enable
> on that filesystem, i.e. XFS:
> 
>         if (ip->i_d.di_flags & XFS_DIFLAG_IMMUTABLE)
>                 stat->attributes |= STATX_ATTR_IMMUTABLE;
>         if (ip->i_d.di_flags & XFS_DIFLAG_APPEND)
>                 stat->attributes |= STATX_ATTR_APPEND;
>         if (ip->i_d.di_flags & XFS_DIFLAG_NODUMP)
>                 stat->attributes |= STATX_ATTR_NODUMP;
> 
>         stat->attributes_mask |= (STATX_ATTR_IMMUTABLE |
>                                   STATX_ATTR_APPEND |
>                                   STATX_ATTR_NODUMP);
> 
> btrfs, cifs, erofs, ext4, f2fs, hfsplus, orangefs and ubifs are similar.
> 
> But others seem to set the mask to everything it can definitively answer,
> i.e. "Encryption and compression are off, and we really mean it" even though
> it will never be set to one in ->attributes, i.e. on gfs2:
> 
>         if (gfsflags & GFS2_DIF_APPENDONLY)
>                 stat->attributes |= STATX_ATTR_APPEND;
>         if (gfsflags & GFS2_DIF_IMMUTABLE)
>                 stat->attributes |= STATX_ATTR_IMMUTABLE;
> 
>         stat->attributes_mask |= (STATX_ATTR_APPEND |
>                                   STATX_ATTR_COMPRESSED |
>                                   STATX_ATTR_ENCRYPTED |
>                                   STATX_ATTR_IMMUTABLE |
>                                   STATX_ATTR_NODUMP);
> 
> ext2 is similar (it adds STATX_ATTR_ENCRYPTED to the mask but will never set
> it in attributes)
> 
> The commit 3209f68b3ca4 which added attributes_mask says:
> 
> "Include a mask in struct stat to indicate which bits of stx_attributes the
> filesystem actually supports."
> 
> The manpage says:
> 
> "A mask indicating which bits in stx_attributes are supported by the VFS and
> the filesystem."
> 
> -and-
> 
> "Note that any attribute that is not indicated as supported by stx_attributes_mask
> has no usable value here."
> 
> So is this intended to indicate which bits of statx->attributes are valid, whether
> they are 1 or 0, or which bits could possibly be set to 1 by the filesystem?
> 
> If the former, then we should move attributes_mask into the VFS to set all flags
> known by the kernel, but David's original commit did not do that so I'm left
> wondering...

Personally I thought that attributes_mask tells you which bits actually
make any sense for the given filesystem, which means:

mask=1 bit=0: "attribute not set on this file"
mask=1 bit=1: "attribute is set on this file"
mask=0 bit=0: "attribute doesn't fit into the design of this fs"
mask=0 bit=1: "filesystem is lying snake"

It's up to the fs driver and not the vfs to set attributes_mask, and
therefore (as I keep pointing out to XiaoLi Feng) xfs_vn_getattr should
be setting the mask.

--D

> 
> Thanks,
> -Eric