On Tue, May 06, 2025 at 07:02:39AM +0200, Christoph Hellwig wrote: > On Mon, May 05, 2025 at 07:29:45AM -0700, Darrick J. Wong wrote: > > attributes_mask contains attribute flags known to the filesystem, > > whereas attributes contains flags actually set on the file. > > "known_attributes" would have been a better name, but that's water under > > the bridge. :P > > Oooh. I think I was very confused at what this patch does, and what > it does seems confused as well. > > The patch adds a new flag to the STATX_ATTR_* namespace, which > historically was used for persistent on-disk flags like immutable, > not the STATX_* namespace where I assumed it, and which has no > support mask. Which seems really odd for a pure kernel feature. > Then again it seems to follow STATX_ATTR_WRITE_ATOMIC which seems > just as wrongly place unless I'm missing something?
I think STATX_* (i.e. not STATX_ATTR_*) flags have two purposes: 1) to declare that specific fields in struct statx actually have meaning, most notably in scenarios where zeroes are valid field contents; and 2) if filling out the field is expensive, userspace can elect not to have it filled by leaving the bit unset. I don't know how userspace is supposed to figure out which fields are expensive. STATX_ATTR_* are supposed to be reflect persistent inode state. I think STATX_ATTR_WRITE_ATOMIC is a (now unremovable) artifact of the era when we were going to have a new iflag and feature bit for all the new forcealign functionality. For XFS it's not necessary anymore because we always have software fallback and the statx::atomic_write_* fields being nonzero is sufficient to detect the functionality. (I'm confused about the whole premise of /this/ patch -- it's a "fast zeroing" fallocate flag that causes the *device* to unmap, so that the filesystem can preallocate and avoid unwritten extent conversions? What happens if the block device is thinp and it runs out of space? That seems antithetical to fallocate...) --D
