* [PATCH 0/1] Baby steps towards removing fuse_launder_folio @ 2025-07-03 19:24 Matthew Wilcox (Oracle) 2025-07-03 19:24 ` [PATCH 1/1] fuse: Use filemap_invalidate_pages() Matthew Wilcox (Oracle) 0 siblings, 1 reply; 5+ messages in thread From: Matthew Wilcox (Oracle) @ 2025-07-03 19:24 UTC (permalink / raw) To: Miklos Szeredi Cc: Matthew Wilcox (Oracle), Joanne Koong, Christoph Hellwig, linux-fsdevel In looking at removing ->launder_folio() altogether, I noticed that FUSE relies on it to write back dirty folios by calling invalidate_inode_pages2() or invalidate_inode_pages2_range() directly. I have formed an opinion that neither of these APIs should be exported to modules; modules should be forced to use filemap_invalidate_pages() instead. It's more efficient because it does the writeback all-at-once. There are other filesystems which use those APIs (notably btrfs), so they're not going away immediately, but I thought I'd get some feedback on this step before I go any further. Matthew Wilcox (Oracle) (1): fuse: Use filemap_invalidate_pages() fs/fuse/dax.c | 16 ++++------------ fs/fuse/dir.c | 12 +++++++----- fs/fuse/file.c | 16 +++++----------- fs/fuse/inode.c | 17 +++++------------ 4 files changed, 21 insertions(+), 40 deletions(-) -- 2.47.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] fuse: Use filemap_invalidate_pages() 2025-07-03 19:24 [PATCH 0/1] Baby steps towards removing fuse_launder_folio Matthew Wilcox (Oracle) @ 2025-07-03 19:24 ` Matthew Wilcox (Oracle) 2025-07-04 9:27 ` kernel test robot ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Matthew Wilcox (Oracle) @ 2025-07-03 19:24 UTC (permalink / raw) To: Miklos Szeredi Cc: Matthew Wilcox (Oracle), Joanne Koong, Christoph Hellwig, linux-fsdevel FUSE relies on invalidate_inode_pages2() / invalidate_inode_pages2_range() doing writeback by calling fuse_launder_folio(). While this works, it is inefficient as each page is written back and waited for individually. Far better to call filemap_invalidate_pages() which will do a bulk write first, then remove the page cache. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/fuse/dax.c | 16 ++++------------ fs/fuse/dir.c | 12 +++++++----- fs/fuse/file.c | 16 +++++----------- fs/fuse/inode.c | 17 +++++------------ 4 files changed, 21 insertions(+), 40 deletions(-) diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c index ac6d4c1064cc..160178b2fce6 100644 --- a/fs/fuse/dax.c +++ b/fs/fuse/dax.c @@ -835,19 +835,11 @@ static int dmap_writeback_invalidate(struct inode *inode, loff_t start_pos = dmap->itn.start << FUSE_DAX_SHIFT; loff_t end_pos = (start_pos + FUSE_DAX_SZ - 1); - ret = filemap_fdatawrite_range(inode->i_mapping, start_pos, end_pos); - if (ret) { - pr_debug("fuse: filemap_fdatawrite_range() failed. err=%d start_pos=0x%llx, end_pos=0x%llx\n", - ret, start_pos, end_pos); - return ret; - } - - ret = invalidate_inode_pages2_range(inode->i_mapping, - start_pos >> PAGE_SHIFT, - end_pos >> PAGE_SHIFT); + ret = filemap_invalidate_pages(inode->i_mapping, start_pos, end_pos, + false); if (ret) - pr_debug("fuse: invalidate_inode_pages2_range() failed err=%d\n", - ret); + pr_debug("fuse: filemap_invalidate_pages() failed. err=%d start_pos=0x%llx, end_pos=0x%llx\n", + ret, start_pos, end_pos); return ret; } diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 2d817d7cab26..0151343d8393 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -718,7 +718,8 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir, if (fm->fc->atomic_o_trunc && trunc) truncate_pagecache(inode, 0); else if (!(ff->open_flags & FOPEN_KEEP_CACHE)) - invalidate_inode_pages2(inode->i_mapping); + filemap_invalidate_pages(inode->i_mapping, 0, + OFFSET_MAX, false); } return err; @@ -1715,7 +1716,8 @@ static int fuse_dir_open(struct inode *inode, struct file *file) if (ff->open_flags & (FOPEN_STREAM | FOPEN_NONSEEKABLE)) nonseekable_open(inode, file); if (!(ff->open_flags & FOPEN_KEEP_CACHE)) - invalidate_inode_pages2(inode->i_mapping); + filemap_invalidate_pages(inode->i_mapping, 0, + OFFSET_MAX, false); } return err; @@ -2088,13 +2090,13 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry, spin_unlock(&fi->lock); /* - * Only call invalidate_inode_pages2() after removing - * FUSE_NOWRITE, otherwise fuse_launder_folio() would deadlock. + * Only call filemap_invalidate_pages() after removing + * FUSE_NOWRITE, otherwise it would deadlock. */ if ((is_truncate || !is_wb) && S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) { truncate_pagecache(inode, outarg.attr.size); - invalidate_inode_pages2(mapping); + filemap_invalidate_pages(mapping, 0, OFFSET_MAX, false); } clear_bit(FUSE_I_SIZE_UNSTABLE, &fi->state); diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 2b04a142b493..eaa659c08132 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -277,7 +277,8 @@ static int fuse_open(struct inode *inode, struct file *file) if (is_truncate) truncate_pagecache(inode, 0); else if (!(ff->open_flags & FOPEN_KEEP_CACHE)) - invalidate_inode_pages2(inode->i_mapping); + filemap_invalidate_pages(inode->i_mapping, 0, + OFFSET_MAX, false); } if (dax_truncate) filemap_invalidate_unlock(inode->i_mapping); @@ -1566,7 +1567,8 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter, return -ENOMEM; if (fopen_direct_io && fc->direct_io_allow_mmap) { - res = filemap_write_and_wait_range(mapping, pos, pos + count - 1); + res = filemap_invalidate_pages(mapping, pos, (pos + count - 1), + false); if (res) { fuse_io_free(ia); return res; @@ -1580,14 +1582,6 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter, inode_unlock(inode); } - if (fopen_direct_io && write) { - res = invalidate_inode_pages2_range(mapping, idx_from, idx_to); - if (res) { - fuse_io_free(ia); - return res; - } - } - io->should_dirty = !write && user_backed_iter(iter); while (count) { ssize_t nres; @@ -2358,7 +2352,7 @@ static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma) if ((vma->vm_flags & VM_MAYSHARE) && !fc->direct_io_allow_mmap) return -ENODEV; - invalidate_inode_pages2(file->f_mapping); + filemap_invalidate_pages(file->f_mapping, 0, OFFSET_MAX, false); if (!(vma->vm_flags & VM_MAYSHARE)) { /* MAP_PRIVATE */ diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index ecb869e895ab..905b192fa12e 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -397,7 +397,8 @@ static void fuse_change_attributes_i(struct inode *inode, struct fuse_attr *attr } if (inval) - invalidate_inode_pages2(inode->i_mapping); + filemap_invalidate_pages(inode->i_mapping, 0, + OFFSET_MAX, false); } if (IS_ENABLED(CONFIG_FUSE_DAX)) @@ -559,8 +560,6 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, { struct fuse_inode *fi; struct inode *inode; - pgoff_t pg_start; - pgoff_t pg_end; inode = fuse_ilookup(fc, nodeid, NULL); if (!inode) @@ -573,15 +572,9 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, fuse_invalidate_attr(inode); forget_all_cached_acls(inode); - if (offset >= 0) { - pg_start = offset >> PAGE_SHIFT; - if (len <= 0) - pg_end = -1; - else - pg_end = (offset + len - 1) >> PAGE_SHIFT; - invalidate_inode_pages2_range(inode->i_mapping, - pg_start, pg_end); - } + if (offset >= 0) + filemap_invalidate_pages(inode->i_mapping, offset, + offset + len - 1, false); iput(inode); return 0; } -- 2.47.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] fuse: Use filemap_invalidate_pages() 2025-07-03 19:24 ` [PATCH 1/1] fuse: Use filemap_invalidate_pages() Matthew Wilcox (Oracle) @ 2025-07-04 9:27 ` kernel test robot 2025-07-04 18:55 ` kernel test robot 2025-07-07 22:07 ` Joanne Koong 2 siblings, 0 replies; 5+ messages in thread From: kernel test robot @ 2025-07-04 9:27 UTC (permalink / raw) To: Matthew Wilcox (Oracle), Miklos Szeredi Cc: oe-kbuild-all, Matthew Wilcox (Oracle), Joanne Koong, Christoph Hellwig, linux-fsdevel Hi Matthew, kernel test robot noticed the following build warnings: [auto build test WARNING on mszeredi-fuse/for-next] [also build test WARNING on linus/master v6.16-rc4 next-20250703] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/fuse-Use-filemap_invalidate_pages/20250704-032629 base: https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next patch link: https://lore.kernel.org/r/20250703192459.3381327-2-willy%40infradead.org patch subject: [PATCH 1/1] fuse: Use filemap_invalidate_pages() config: microblaze-randconfig-r073-20250704 (https://download.01.org/0day-ci/archive/20250704/202507041729.OkMQMi8u-lkp@intel.com/config) compiler: microblaze-linux-gcc (GCC) 8.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250704/202507041729.OkMQMi8u-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507041729.OkMQMi8u-lkp@intel.com/ All warnings (new ones prefixed by >>): fs/fuse/file.c: In function 'fuse_direct_io': >> fs/fuse/file.c:1557:10: warning: unused variable 'idx_to' [-Wunused-variable] pgoff_t idx_to = (pos + count - 1) >> PAGE_SHIFT; ^~~~~~ >> fs/fuse/file.c:1556:10: warning: unused variable 'idx_from' [-Wunused-variable] pgoff_t idx_from = pos >> PAGE_SHIFT; ^~~~~~~~ vim +/idx_to +1557 fs/fuse/file.c 413ef8cb302511 Miklos Szeredi 2005-09-09 1542 d22a943f44c79c Al Viro 2014-03-16 1543 ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter, d22a943f44c79c Al Viro 2014-03-16 1544 loff_t *ppos, int flags) 413ef8cb302511 Miklos Szeredi 2005-09-09 1545 { ea8cd33390fafc Pavel Emelyanov 2013-10-10 1546 int write = flags & FUSE_DIO_WRITE; ea8cd33390fafc Pavel Emelyanov 2013-10-10 1547 int cuse = flags & FUSE_DIO_CUSE; e1c0eecba1a415 Miklos Szeredi 2017-09-12 1548 struct file *file = io->iocb->ki_filp; 80e4f25262f9f1 Hao Xu 2023-08-01 1549 struct address_space *mapping = file->f_mapping; 80e4f25262f9f1 Hao Xu 2023-08-01 1550 struct inode *inode = mapping->host; 2106cb18930312 Miklos Szeredi 2009-04-28 1551 struct fuse_file *ff = file->private_data; fcee216beb9c15 Max Reitz 2020-05-06 1552 struct fuse_conn *fc = ff->fm->fc; 413ef8cb302511 Miklos Szeredi 2005-09-09 1553 size_t nmax = write ? fc->max_write : fc->max_read; 413ef8cb302511 Miklos Szeredi 2005-09-09 1554 loff_t pos = *ppos; d22a943f44c79c Al Viro 2014-03-16 1555 size_t count = iov_iter_count(iter); 09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 @1556 pgoff_t idx_from = pos >> PAGE_SHIFT; 09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 @1557 pgoff_t idx_to = (pos + count - 1) >> PAGE_SHIFT; 413ef8cb302511 Miklos Szeredi 2005-09-09 1558 ssize_t res = 0; 742f992708dff0 Ashish Samant 2016-03-14 1559 int err = 0; 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1560 struct fuse_io_args *ia; 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1561 unsigned int max_pages; 80e4f25262f9f1 Hao Xu 2023-08-01 1562 bool fopen_direct_io = ff->open_flags & FOPEN_DIRECT_IO; 248d86e87d12da Miklos Szeredi 2006-01-06 1563 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1564 max_pages = iov_iter_npages(iter, fc->max_pages); 68bfb7eb7f7de3 Joanne Koong 2024-10-24 1565 ia = fuse_io_alloc(io, max_pages); 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1566 if (!ia) 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1567 return -ENOMEM; 413ef8cb302511 Miklos Szeredi 2005-09-09 1568 c55e0a55b16520 Tyler Fanelli 2023-09-19 1569 if (fopen_direct_io && fc->direct_io_allow_mmap) { 147c1229fcb032 Matthew Wilcox (Oracle 2025-07-03 1570) res = filemap_invalidate_pages(mapping, pos, (pos + count - 1), 147c1229fcb032 Matthew Wilcox (Oracle 2025-07-03 1571) false); b5a2a3a0b77668 Hao Xu 2023-08-01 1572 if (res) { 68bfb7eb7f7de3 Joanne Koong 2024-10-24 1573 fuse_io_free(ia); b5a2a3a0b77668 Hao Xu 2023-08-01 1574 return res; b5a2a3a0b77668 Hao Xu 2023-08-01 1575 } b5a2a3a0b77668 Hao Xu 2023-08-01 1576 } 0c58a97f919c24 Joanne Koong 2025-04-14 1577 if (!cuse && filemap_range_has_writeback(mapping, pos, (pos + count - 1))) { ea8cd33390fafc Pavel Emelyanov 2013-10-10 1578 if (!write) 5955102c9984fa Al Viro 2016-01-22 1579 inode_lock(inode); ea8cd33390fafc Pavel Emelyanov 2013-10-10 1580 fuse_sync_writes(inode); ea8cd33390fafc Pavel Emelyanov 2013-10-10 1581 if (!write) 5955102c9984fa Al Viro 2016-01-22 1582 inode_unlock(inode); ea8cd33390fafc Pavel Emelyanov 2013-10-10 1583 } ea8cd33390fafc Pavel Emelyanov 2013-10-10 1584 fcb14cb1bdacec Al Viro 2022-05-22 1585 io->should_dirty = !write && user_backed_iter(iter); 413ef8cb302511 Miklos Szeredi 2005-09-09 1586 while (count) { 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1587 ssize_t nres; 2106cb18930312 Miklos Szeredi 2009-04-28 1588 fl_owner_t owner = current->files; f4975c67dd9ad8 Miklos Szeredi 2009-04-02 1589 size_t nbytes = min(count, nmax); 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1590 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1591 err = fuse_get_user_pages(&ia->ap, iter, &nbytes, write, 41748675c0bf25 Hou Tao 2024-08-31 1592 max_pages, fc->use_pages_for_kvec_io); 742f992708dff0 Ashish Samant 2016-03-14 1593 if (err && !nbytes) 413ef8cb302511 Miklos Szeredi 2005-09-09 1594 break; f4975c67dd9ad8 Miklos Szeredi 2009-04-02 1595 4a2abf99f9c287 Miklos Szeredi 2019-05-27 1596 if (write) { 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1597 if (!capable(CAP_FSETID)) 10c52c84e3f487 Miklos Szeredi 2020-11-11 1598 ia->write.in.write_flags |= FUSE_WRITE_KILL_SUIDGID; 4a2abf99f9c287 Miklos Szeredi 2019-05-27 1599 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1600 nres = fuse_send_write(ia, pos, nbytes, owner); 4a2abf99f9c287 Miklos Szeredi 2019-05-27 1601 } else { 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1602 nres = fuse_send_read(ia, pos, nbytes, owner); 4a2abf99f9c287 Miklos Szeredi 2019-05-27 1603 } 2106cb18930312 Miklos Szeredi 2009-04-28 1604 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1605 if (!io->async || nres < 0) { 41748675c0bf25 Hou Tao 2024-08-31 1606 fuse_release_user_pages(&ia->ap, nres, io->should_dirty); 68bfb7eb7f7de3 Joanne Koong 2024-10-24 1607 fuse_io_free(ia); 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1608 } 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1609 ia = NULL; 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1610 if (nres < 0) { f658adeea45e43 Miklos Szeredi 2020-02-06 1611 iov_iter_revert(iter, nbytes); 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1612 err = nres; 413ef8cb302511 Miklos Szeredi 2005-09-09 1613 break; 413ef8cb302511 Miklos Szeredi 2005-09-09 1614 } 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1615 WARN_ON(nres > nbytes); 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1616 413ef8cb302511 Miklos Szeredi 2005-09-09 1617 count -= nres; 413ef8cb302511 Miklos Szeredi 2005-09-09 1618 res += nres; 413ef8cb302511 Miklos Szeredi 2005-09-09 1619 pos += nres; f658adeea45e43 Miklos Szeredi 2020-02-06 1620 if (nres != nbytes) { f658adeea45e43 Miklos Szeredi 2020-02-06 1621 iov_iter_revert(iter, nbytes - nres); 413ef8cb302511 Miklos Szeredi 2005-09-09 1622 break; f658adeea45e43 Miklos Szeredi 2020-02-06 1623 } 56cf34ff079569 Miklos Szeredi 2006-04-11 1624 if (count) { 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1625 max_pages = iov_iter_npages(iter, fc->max_pages); 68bfb7eb7f7de3 Joanne Koong 2024-10-24 1626 ia = fuse_io_alloc(io, max_pages); 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1627 if (!ia) 56cf34ff079569 Miklos Szeredi 2006-04-11 1628 break; 56cf34ff079569 Miklos Szeredi 2006-04-11 1629 } 413ef8cb302511 Miklos Szeredi 2005-09-09 1630 } 45ac96ed7c3691 Miklos Szeredi 2019-09-10 1631 if (ia) 68bfb7eb7f7de3 Joanne Koong 2024-10-24 1632 fuse_io_free(ia); d09cb9d7f6e4cb Miklos Szeredi 2009-04-28 1633 if (res > 0) 413ef8cb302511 Miklos Szeredi 2005-09-09 1634 *ppos = pos; 413ef8cb302511 Miklos Szeredi 2005-09-09 1635 742f992708dff0 Ashish Samant 2016-03-14 1636 return res > 0 ? res : err; 413ef8cb302511 Miklos Szeredi 2005-09-09 1637 } 08cbf542bf24fb Tejun Heo 2009-04-14 1638 EXPORT_SYMBOL_GPL(fuse_direct_io); 413ef8cb302511 Miklos Szeredi 2005-09-09 1639 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] fuse: Use filemap_invalidate_pages() 2025-07-03 19:24 ` [PATCH 1/1] fuse: Use filemap_invalidate_pages() Matthew Wilcox (Oracle) 2025-07-04 9:27 ` kernel test robot @ 2025-07-04 18:55 ` kernel test robot 2025-07-07 22:07 ` Joanne Koong 2 siblings, 0 replies; 5+ messages in thread From: kernel test robot @ 2025-07-04 18:55 UTC (permalink / raw) To: Matthew Wilcox (Oracle), Miklos Szeredi Cc: llvm, oe-kbuild-all, Matthew Wilcox (Oracle), Joanne Koong, Christoph Hellwig, linux-fsdevel Hi Matthew, kernel test robot noticed the following build errors: [auto build test ERROR on mszeredi-fuse/for-next] [also build test ERROR on linus/master v6.16-rc4 next-20250704] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/fuse-Use-filemap_invalidate_pages/20250704-032629 base: https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next patch link: https://lore.kernel.org/r/20250703192459.3381327-2-willy%40infradead.org patch subject: [PATCH 1/1] fuse: Use filemap_invalidate_pages() config: loongarch-defconfig (https://download.01.org/0day-ci/archive/20250705/202507050209.rVvcbaHY-lkp@intel.com/config) compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507050209.rVvcbaHY-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507050209.rVvcbaHY-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "filemap_invalidate_pages" [fs/fuse/fuse.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] fuse: Use filemap_invalidate_pages() 2025-07-03 19:24 ` [PATCH 1/1] fuse: Use filemap_invalidate_pages() Matthew Wilcox (Oracle) 2025-07-04 9:27 ` kernel test robot 2025-07-04 18:55 ` kernel test robot @ 2025-07-07 22:07 ` Joanne Koong 2 siblings, 0 replies; 5+ messages in thread From: Joanne Koong @ 2025-07-07 22:07 UTC (permalink / raw) To: Matthew Wilcox (Oracle); +Cc: Miklos Szeredi, Christoph Hellwig, linux-fsdevel On Thu, Jul 3, 2025 at 12:25 PM Matthew Wilcox (Oracle) <willy@infradead.org> wrote: > > FUSE relies on invalidate_inode_pages2() / invalidate_inode_pages2_range() > doing writeback by calling fuse_launder_folio(). While this works, it > is inefficient as each page is written back and waited for individually. > Far better to call filemap_invalidate_pages() which will do a bulk write > first, then remove the page cache. This logic makes sense to me but what about the case where writeback errors out? With invalidate_inode_pages2() / invalidate_inode_pages2_range(), the pages still get unmapped, but with filemap_invalidate_pages(), this will return an error without unmapping. AFAICT, I think we would still want the unmapping to be done if there's an error in writeback. I'm a bit confused about the difference between "each page is written back and waited for individually" vs bulk write. AFAICT, with the bulk write, which calls ->writepages(), in fuse this will still write each page/folio individually through write_cache_pages() and AFAICT, each page/folio is also waited for individually in __filemap_fdatawait_range(). With doing the bulk write, I wonder if this sometimes could be more inefficient than doing the writeback only after it's been unmapped, eg if there's writes after the filemap_invalidate_pages() writeback, we'd end up doing 2 writebacks. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/fuse/dax.c | 16 ++++------------ > fs/fuse/dir.c | 12 +++++++----- > fs/fuse/file.c | 16 +++++----------- > fs/fuse/inode.c | 17 +++++------------ > 4 files changed, 21 insertions(+), 40 deletions(-) > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c > index 2d817d7cab26..0151343d8393 100644 > --- a/fs/fuse/dir.c > +++ b/fs/fuse/dir.c > @@ -718,7 +718,8 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir, > if (fm->fc->atomic_o_trunc && trunc) > truncate_pagecache(inode, 0); > else if (!(ff->open_flags & FOPEN_KEEP_CACHE)) > - invalidate_inode_pages2(inode->i_mapping); > + filemap_invalidate_pages(inode->i_mapping, 0, > + OFFSET_MAX, false); nit (here and below): in fuse, the line break spacing for args is aligned to be right underneath the first arg > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 2b04a142b493..eaa659c08132 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -1566,7 +1567,8 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter, > return -ENOMEM; > > if (fopen_direct_io && fc->direct_io_allow_mmap) { > - res = filemap_write_and_wait_range(mapping, pos, pos + count - 1); > + res = filemap_invalidate_pages(mapping, pos, (pos + count - 1), nit: don't need the parentheses around pos + count - 1 > + false); > if (res) { > fuse_io_free(ia); > return res; > @@ -1580,14 +1582,6 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter, > inode_unlock(inode); > } > > - if (fopen_direct_io && write) { > - res = invalidate_inode_pages2_range(mapping, idx_from, idx_to); > - if (res) { > - fuse_io_free(ia); > - return res; > - } > - } I'm having trouble understanding why we can get rid of this logic here. Is this because we now do filemap_invalidate_pages() for the "if (fopen_direct_io && fc->direct_io_allow_mmap)" part above? AFAIS if fc->direct_io_allow_mmap is false and we're handling a write, we still need to call invalidate_inode_pages2_range()/filemap_invalidate_pages() here, no? > - > io->should_dirty = !write && user_backed_iter(iter); > while (count) { > ssize_t nres; > @@ -2358,7 +2352,7 @@ static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma) > if ((vma->vm_flags & VM_MAYSHARE) && !fc->direct_io_allow_mmap) > return -ENODEV; > > - invalidate_inode_pages2(file->f_mapping); > + filemap_invalidate_pages(file->f_mapping, 0, OFFSET_MAX, false); > > if (!(vma->vm_flags & VM_MAYSHARE)) { > /* MAP_PRIVATE */ > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index ecb869e895ab..905b192fa12e 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -559,8 +560,6 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, > { > struct fuse_inode *fi; > struct inode *inode; > - pgoff_t pg_start; > - pgoff_t pg_end; > > inode = fuse_ilookup(fc, nodeid, NULL); > if (!inode) > @@ -573,15 +572,9 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, > > fuse_invalidate_attr(inode); > forget_all_cached_acls(inode); > - if (offset >= 0) { > - pg_start = offset >> PAGE_SHIFT; > - if (len <= 0) > - pg_end = -1; Should we preserve this len <= 0 logic? The len value comes from the server. If they pass in -1, that means they want the entire file invalidated. Thanks, Joanne > - else > - pg_end = (offset + len - 1) >> PAGE_SHIFT; > - invalidate_inode_pages2_range(inode->i_mapping, > - pg_start, pg_end); > - } > + if (offset >= 0) > + filemap_invalidate_pages(inode->i_mapping, offset, > + offset + len - 1, false); > iput(inode); > return 0; > } > -- > 2.47.2 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-07 22:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-03 19:24 [PATCH 0/1] Baby steps towards removing fuse_launder_folio Matthew Wilcox (Oracle) 2025-07-03 19:24 ` [PATCH 1/1] fuse: Use filemap_invalidate_pages() Matthew Wilcox (Oracle) 2025-07-04 9:27 ` kernel test robot 2025-07-04 18:55 ` kernel test robot 2025-07-07 22:07 ` Joanne Koong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).