Re: [PATCH] fs/ntfs3: Add missing direct_IO in ntfs_aops_cmpr

[Lizhi Xu]

On Sun, 13 Apr 2025 22:50:54 -0700, Christoph Hellwig wrote:
> On Fri, Apr 11, 2025 at 09:24:27AM +0800, Lizhi Xu wrote:
> > The ntfs3 can use the page cache directly, so its address_space_operations
> > need direct_IO.
> 
> I can't parse that sentence.  What are you trying to say with it?
The comments [1] of generic_file_read_iter() clearly states "read_iter()
for all filesystems that can use the page cache directly".

In the calltrace of this example, it is clear that direct_IO is not set.
In [3], it is also clear that the lack of direct_IO in ntfs_aops_cmpr
caused this problem.

In summary, direct_IO must be set in this issue.

[1]
 * generic_file_read_iter - generic filesystem read routine
 * @iocb:	kernel I/O control block
 * @iter:	destination for the data read
 *
 * This is the "read_iter()" routine for all filesystems
 * that can use the page cache directly.

[2]
generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
{
	size_t count = iov_iter_count(iter);
	ssize_t retval = 0;

	if (!count)
		return 0; /* skip atime */

	if (iocb->ki_flags & IOCB_DIRECT) {
		struct file *file = iocb->ki_filp;
		struct address_space *mapping = file->f_mapping;
		struct inode *inode = mapping->host;

		retval = kiocb_write_and_wait(iocb, count);
		if (retval < 0)
			return retval;
		file_accessed(file);

		retval = mapping->a_ops->direct_IO(iocb, iter); 
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b432163ebd15a0fb74051949cb61456d6c55ccbd
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 4d9d84cc3c6f55..9b6a3f8d2e7c5c 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -101,8 +101,26 @@ int ntfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
 	/* Allowed to change compression for empty files and for directories only. */
 	if (!is_dedup(ni) && !is_encrypted(ni) &&
 	    (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) {
-		/* Change compress state. */
-		int err = ni_set_compress(inode, flags & FS_COMPR_FL);
+		int err = 0;
+		struct address_space *mapping = inode->i_mapping;
+
+		/* write out all data and wait. */
+		filemap_invalidate_lock(mapping);
+		err = filemap_write_and_wait(mapping);
+
+		if (err >= 0) {
+			/* Change compress state. */
+			bool compr = flags & FS_COMPR_FL;
+			err = ni_set_compress(inode, compr);
+
+			/* For files change a_ops too. */
+			if (!err)
+				mapping->a_ops = compr ? &ntfs_aops_cmpr :
+							 &ntfs_aops;

BR,
Lizhi