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

Lizhi Xu
2025-04-15T01:05:49Z

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