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
