| Message ID | 20250415092637.251786-1-lizhi.xu@windriver.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] fs/ntfs3: Add missing direct_IO in ntfs_aops_cmpr | expand |
On Tue 15-04-25 17:26:37, Lizhi Xu wrote: > The ntfs3 can use the page cache directly, so its address_space_operations > need direct_IO. Exit ntfs_direct_IO() if it is a compressed file. > > Fixes: b432163ebd15 ("fs/ntfs3: Update inode->i_mapping->a_ops on compression state") > Reported-by: syzbot+e36cc3297bd3afd25e19@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=e36cc3297bd3afd25e19 > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> OK, this looks sensible to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > V1 -> V2: exit direct io if it is a compressed file. > > fs/ntfs3/inode.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c > index 3e2957a1e360..0f0d27d4644a 100644 > --- a/fs/ntfs3/inode.c > +++ b/fs/ntfs3/inode.c > @@ -805,6 +805,10 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > ret = 0; > goto out; > } > + if (is_compressed(ni)) { > + ret = 0; > + goto out; > + } > > ret = blockdev_direct_IO(iocb, inode, iter, > wr ? ntfs_get_block_direct_IO_W : > @@ -2068,5 +2072,6 @@ const struct address_space_operations ntfs_aops_cmpr = { > .read_folio = ntfs_read_folio, > .readahead = ntfs_readahead, > .dirty_folio = block_dirty_folio, > + .direct_IO = ntfs_direct_IO, > }; > // clang-format on > -- > 2.43.0 >
On Tue, Apr 15, 2025 at 05:26:37PM +0800, Lizhi Xu wrote: > The ntfs3 can use the page cache directly, so its address_space_operations > need direct_IO. This sentence still does not make any sense.
On Tue, 15 Apr 2025 21:37:04 -0700, Christoph Hellwig wrote: > > The ntfs3 can use the page cache directly, so its address_space_operations > > need direct_IO. > > This sentence still does not make any sense. Did you see the following comments? https://lore.kernel.org/all/20250415010518.2008216-1-lizhi.xu@windriver.com/
On Wed, Apr 16, 2025 at 01:34:26PM +0800, Lizhi Xu wrote: > On Tue, 15 Apr 2025 21:37:04 -0700, Christoph Hellwig wrote: > > > The ntfs3 can use the page cache directly, so its address_space_operations > > > need direct_IO. > > > > This sentence still does not make any sense. > Did you see the following comments? > https://lore.kernel.org/all/20250415010518.2008216-1-lizhi.xu@windriver.com/ I did, but that changes nothing about the fact that the above sentence doesn't make sense.
On Tue, 15 Apr 2025 22:35:30 -0700, Christoph Hellwig wrote: > > > > The ntfs3 can use the page cache directly, so its address_space_operations > > > > need direct_IO. > > > > > > This sentence still does not make any sense. > > Did you see the following comments? > > https://lore.kernel.org/all/20250415010518.2008216-1-lizhi.xu@windriver.com/ > > I did, but that changes nothing about the fact that the above sentence > doesn't make sense. In the reproducer, the second file passed in by the system call sendfile() sets the file flag O_DIRECT when opening the file, which bypasses the page cache and accesses the direct io interface of the ntfs3 file system. However, ntfs3 does not set direct_IO for compressed files in ntfs_aops_cmpr.
On Wed, Apr 16, 2025 at 02:03:51PM +0800, Lizhi Xu wrote: > In the reproducer, the second file passed in by the system call sendfile() > sets the file flag O_DIRECT when opening the file, which bypasses the page > cache and accesses the direct io interface of the ntfs3 file system. > However, ntfs3 does not set direct_IO for compressed files in ntfs_aops_cmpr. Not allowing direct I/O is perfectly fine. If you think you need to support direct I/O for this case it is also fine. But none of this has anything to do with 'can use the page cache' and there are also plenty of ways to support direct I/O without ->direct_IO.
On Tue, 15 Apr 2025 23:05:56 -0700, Christoph Hellwig wrote: > > In the reproducer, the second file passed in by the system call sendfile() > > sets the file flag O_DIRECT when opening the file, which bypasses the page > > cache and accesses the direct io interface of the ntfs3 file system. > > However, ntfs3 does not set direct_IO for compressed files in ntfs_aops_cmpr. > > Not allowing direct I/O is perfectly fine. If you think you need to > support direct I/O for this case it is also fine. But none of this > has anything to do with 'can use the page cache' and there are also The "The ntfs3 can use the page cache directly" I mentioned in the patch is to explain that the calltrace is the direct I/O of ntfs3 called from generic_file_read_iter().
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index 3e2957a1e360..0f0d27d4644a 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -805,6 +805,10 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) ret = 0; goto out; } + if (is_compressed(ni)) { + ret = 0; + goto out; + } ret = blockdev_direct_IO(iocb, inode, iter, wr ? ntfs_get_block_direct_IO_W : @@ -2068,5 +2072,6 @@ const struct address_space_operations ntfs_aops_cmpr = { .read_folio = ntfs_read_folio, .readahead = ntfs_readahead, .dirty_folio = block_dirty_folio, + .direct_IO = ntfs_direct_IO, }; // clang-format on
The ntfs3 can use the page cache directly, so its address_space_operations need direct_IO. Exit ntfs_direct_IO() if it is a compressed file. Fixes: b432163ebd15 ("fs/ntfs3: Update inode->i_mapping->a_ops on compression state") Reported-by: syzbot+e36cc3297bd3afd25e19@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=e36cc3297bd3afd25e19 Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 -> V2: exit direct io if it is a compressed file. fs/ntfs3/inode.c | 1 + 1 file changed, 1 insertion(+)