linux-kernel - Re: [PATCH v2 06/11] ntfsplus: add iomap and address space operations

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: <CAKYAXd-Gzk+7Gwh1GTVbeNUygNWVnmNu458F67Y5fhcpapEFBg@mail.gmail.com>
Date: Tue, 2 Dec 2025 16:52:59 +0900
From: Namjae Jeon <linkinjeon@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: viro@...iv.linux.org.uk, brauner@...nel.org, tytso@....edu, 
	willy@...radead.org, jack@...e.cz, djwong@...nel.org, josef@...icpanda.com, 
	sandeen@...deen.net, rgoldwyn@...e.com, xiang@...nel.org, dsterba@...e.com, 
	pali@...nel.org, ebiggers@...nel.org, neil@...wn.name, amir73il@...il.com, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	iamjoonsoo.kim@....com, cheol.lee@....com, jay.sim@....com, gunho.lee@....com, 
	Hyunchul Lee <hyc.lee@...il.com>
Subject: Re: [PATCH v2 06/11] ntfsplus: add iomap and address space operations

On Tue, Dec 2, 2025 at 2:45 PM Christoph Hellwig <hch@....de> wrote:
>
> On Tue, Dec 02, 2025 at 09:47:17AM +0900, Namjae Jeon wrote:
> > Nothing special reason, It was to use existing ones instead of new,
> > complex implementations. NTFS metadata is treated as a file, and
> > handling it via the folio(page) API allows the driver to easily gain
> > performance benefits, such as readahead.
>
> On the one hand it does, on the other hand at least our experience
> is that the user data file algorithm for things like readahead and
> cache eviction policies worked pretty poorly for metadata in XFS.
> Of course I don't actually know if the same applies to ntfs.
We have observed performance improvements from readahead for NTFS
metadata since we are able to identify the continuous cluster ranges
of metadata files.
>
> > > From code in other patches is looks like ntfs never switches between
> > > compressed and non-compressed for live inodes?  In that case the
> > > separate aops should be fine, as switching between them at runtime
> > > would involve races.  Is the compression policy per-directory?
> > Non-compressed files can actually be switched to compressed files and
> > vice versa via setxattr at runtime. I will check the race handling
> > around aop switching again. And the compression policy is per-file,
> > not per-directory.
>
> In that case you probably want to use the same set of address space
> (and other operations) and do runtime switching inside the method.
Right, I will change it.
>
> > >
> > > Again curious why we need special zeroing code in the file system.
> > To prevent reading garbage data after a new cluster allocation, we
> > must zero out the cluster. The cluster size can be up to 2MB, I will
> > check if that's possible through iomap.
>
> Ouch, that's a lot of zeroing.  But yeah, now that you mention it
> XFS actually has the same issue with large RT extents.  Although we
> create them as unwritten extents, i.e. disk allocations that always
> return zeroes.  I guess ntfs doesn't have that.  For DAX access
> there actually is zeroing in the allocator, which is probably
> similar to what is done here, just always using the iomap-based
> code (check for xfs_zero_range and callers).
Right, ntfs does not have a direct equivalent to the unwritten extent mechanism.
I will check xfs codes. Thank you very much for the detailed review!