* [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).