linux-kernel - Re: [RFC PATCH v4 07/11] fs: statx add write zeroes unmap attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250506053654.GA25700@frogsfrogsfrogs>
Date: Mon, 5 May 2025 22:36:54 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Zhang Yi <yi.zhang@...weicloud.com>, linux-fsdevel@...r.kernel.org,
	linux-ext4@...r.kernel.org, linux-block@...r.kernel.org,
	dm-devel@...ts.linux.dev, linux-nvme@...ts.infradead.org,
	linux-scsi@...r.kernel.org, linux-xfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, tytso@....edu,
	john.g.garry@...cle.com, bmarzins@...hat.com, chaitanyak@...dia.com,
	shinichiro.kawasaki@....com, brauner@...nel.org,
	yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com,
	yangerkun@...wei.com
Subject: Re: [RFC PATCH v4 07/11] fs: statx add write zeroes unmap attribute

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