Hi all,
The VM doesn't need or want ->writepage for writeback and is fine with
just having ->writepages as long as ->migrate_folio is implemented.
This series removes all ->writepage instances that use
block_write_full_page directly and also have a plain mpage_writepages
based ->writepages.
Diffstat:
fs/exfat/inode.c | 9 ++-------
fs/ext2/inode.c | 6 ------
fs/fat/inode.c | 9 ++-------
fs/hfs/inode.c | 2 +-
fs/hfsplus/inode.c | 2 +-
fs/hpfs/file.c | 9 ++-------
fs/jfs/inode.c | 7 +------
fs/omfs/file.c | 7 +------
fs/udf/inode.c | 7 +------
9 files changed, 11 insertions(+), 47 deletions(-)
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and stop
wiring up ->writepage for hfs_aops.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/hfs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index c4526f16355d5..16466a5e88b44 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -173,12 +173,12 @@ const struct address_space_operations hfs_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = hfs_read_folio,
- .writepage = hfs_writepage,
.write_begin = hfs_write_begin,
.write_end = generic_write_end,
.bmap = hfs_bmap,
.direct_IO = hfs_direct_IO,
.writepages = hfs_writepages,
+ .migrate_folio = buffer_migrate_folio,
};
/*
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ext2/inode.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 918ab2f9e4c05..3b2e3e1e0fa25 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -869,11 +869,6 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
return ret;
}
-static int ext2_writepage(struct page *page, struct writeback_control *wbc)
-{
- return block_write_full_page(page, ext2_get_block, wbc);
-}
-
static int ext2_read_folio(struct file *file, struct folio *folio)
{
return mpage_read_folio(folio, ext2_get_block);
@@ -948,7 +943,6 @@ const struct address_space_operations ext2_aops = {
.invalidate_folio = block_invalidate_folio,
.read_folio = ext2_read_folio,
.readahead = ext2_readahead,
- .writepage = ext2_writepage,
.write_begin = ext2_write_begin,
.write_end = ext2_write_end,
.bmap = ext2_bmap,
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and remove
the ->writepage implementation.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/fat/inode.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 1cbcc4608dc78..d99b8549ec8f9 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -194,11 +194,6 @@ static int fat_get_block(struct inode *inode, sector_t iblock,
return 0;
}
-static int fat_writepage(struct page *page, struct writeback_control *wbc)
-{
- return block_write_full_page(page, fat_get_block, wbc);
-}
-
static int fat_writepages(struct address_space *mapping,
struct writeback_control *wbc)
{
@@ -346,12 +341,12 @@ static const struct address_space_operations fat_aops = {
.invalidate_folio = block_invalidate_folio,
.read_folio = fat_read_folio,
.readahead = fat_readahead,
- .writepage = fat_writepage,
.writepages = fat_writepages,
.write_begin = fat_write_begin,
.write_end = fat_write_end,
.direct_IO = fat_direct_IO,
- .bmap = _fat_bmap
+ .bmap = _fat_bmap,
+ .migrate_folio = buffer_migrate_folio,
};
/*
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and stop
wiring up ->writepage for hfsplus_aops.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/hfsplus/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index aeab83ed1c9c6..d6572ad2407a7 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -170,12 +170,12 @@ const struct address_space_operations hfsplus_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = hfsplus_read_folio,
- .writepage = hfsplus_writepage,
.write_begin = hfsplus_write_begin,
.write_end = generic_write_end,
.bmap = hfsplus_bmap,
.direct_IO = hfsplus_direct_IO,
.writepages = hfsplus_writepages,
+ .migrate_folio = buffer_migrate_folio,
};
const struct dentry_operations hfsplus_dentry_operations = {
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and remove
the ->writepage implementation.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/hpfs/file.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index f7547a62c81f6..88952d4a631e6 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -163,11 +163,6 @@ static int hpfs_read_folio(struct file *file, struct folio *folio)
return mpage_read_folio(folio, hpfs_get_block);
}
-static int hpfs_writepage(struct page *page, struct writeback_control *wbc)
-{
- return block_write_full_page(page, hpfs_get_block, wbc);
-}
-
static void hpfs_readahead(struct readahead_control *rac)
{
mpage_readahead(rac, hpfs_get_block);
@@ -248,12 +243,12 @@ const struct address_space_operations hpfs_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = hpfs_read_folio,
- .writepage = hpfs_writepage,
.readahead = hpfs_readahead,
.writepages = hpfs_writepages,
.write_begin = hpfs_write_begin,
.write_end = hpfs_write_end,
- .bmap = _hpfs_bmap
+ .bmap = _hpfs_bmap,
+ .migrate_folio = buffer_migrate_folio,
};
const struct file_operations hpfs_file_ops =
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and remove
the ->writepage implementation.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/jfs/inode.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index d1ec920aa030a..8ac10e3960508 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -264,11 +264,6 @@ int jfs_get_block(struct inode *ip, sector_t lblock,
return rc;
}
-static int jfs_writepage(struct page *page, struct writeback_control *wbc)
-{
- return block_write_full_page(page, jfs_get_block, wbc);
-}
-
static int jfs_writepages(struct address_space *mapping,
struct writeback_control *wbc)
{
@@ -355,12 +350,12 @@ const struct address_space_operations jfs_aops = {
.invalidate_folio = block_invalidate_folio,
.read_folio = jfs_read_folio,
.readahead = jfs_readahead,
- .writepage = jfs_writepage,
.writepages = jfs_writepages,
.write_begin = jfs_write_begin,
.write_end = jfs_write_end,
.bmap = jfs_bmap,
.direct_IO = jfs_direct_IO,
+ .migrate_folio = buffer_migrate_folio,
};
/*
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and remove
the ->writepage implementation.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/omfs/file.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/omfs/file.c b/fs/omfs/file.c
index fa7fe2393ff68..3a5b4b88a5838 100644
--- a/fs/omfs/file.c
+++ b/fs/omfs/file.c
@@ -294,11 +294,6 @@ static void omfs_readahead(struct readahead_control *rac)
mpage_readahead(rac, omfs_get_block);
}
-static int omfs_writepage(struct page *page, struct writeback_control *wbc)
-{
- return block_write_full_page(page, omfs_get_block, wbc);
-}
-
static int
omfs_writepages(struct address_space *mapping, struct writeback_control *wbc)
{
@@ -375,10 +370,10 @@ const struct address_space_operations omfs_aops = {
.invalidate_folio = block_invalidate_folio,
.read_folio = omfs_read_folio,
.readahead = omfs_readahead,
- .writepage = omfs_writepage,
.writepages = omfs_writepages,
.write_begin = omfs_write_begin,
.write_end = generic_write_end,
.bmap = omfs_bmap,
+ .migrate_folio = buffer_migrate_folio,
};
--
2.30.2
->writepage is a very inefficient method to write back data, and only
used through write_cache_pages or a a fallback when no ->migrate_folio
method is present.
Set ->migrate_folio to the generic buffer_head based helper, and remove
the ->writepage implementation in extfat.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/udf/inode.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index dce6ae9ae306c..0246b1b86fb91 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -182,11 +182,6 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
}
}
-static int udf_writepage(struct page *page, struct writeback_control *wbc)
-{
- return block_write_full_page(page, udf_get_block, wbc);
-}
-
static int udf_writepages(struct address_space *mapping,
struct writeback_control *wbc)
{
@@ -239,12 +234,12 @@ const struct address_space_operations udf_aops = {
.invalidate_folio = block_invalidate_folio,
.read_folio = udf_read_folio,
.readahead = udf_readahead,
- .writepage = udf_writepage,
.writepages = udf_writepages,
.write_begin = udf_write_begin,
.write_end = generic_write_end,
.direct_IO = udf_direct_IO,
.bmap = udf_bmap,
+ .migrate_folio = buffer_migrate_folio,
};
/*
--
2.30.2
On Sun 13-11-22 17:28:55, Christoph Hellwig wrote:
> ->writepage is a very inefficient method to write back data, and only
> used through write_cache_pages or a a fallback when no ->migrate_folio
> method is present.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
Looks good! Feel free to add:
Acked-by: Jan Kara <[email protected]>
Honza
> ---
> fs/ext2/inode.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 918ab2f9e4c05..3b2e3e1e0fa25 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -869,11 +869,6 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> return ret;
> }
>
> -static int ext2_writepage(struct page *page, struct writeback_control *wbc)
> -{
> - return block_write_full_page(page, ext2_get_block, wbc);
> -}
> -
> static int ext2_read_folio(struct file *file, struct folio *folio)
> {
> return mpage_read_folio(folio, ext2_get_block);
> @@ -948,7 +943,6 @@ const struct address_space_operations ext2_aops = {
> .invalidate_folio = block_invalidate_folio,
> .read_folio = ext2_read_folio,
> .readahead = ext2_readahead,
> - .writepage = ext2_writepage,
> .write_begin = ext2_write_begin,
> .write_end = ext2_write_end,
> .bmap = ext2_bmap,
> --
> 2.30.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Sun 13-11-22 17:29:02, Christoph Hellwig wrote:
> ->writepage is a very inefficient method to write back data, and only
> used through write_cache_pages or a a fallback when no ->migrate_folio
> method is present.
>
> Set ->migrate_folio to the generic buffer_head based helper, and remove
> the ->writepage implementation in extfat.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
Looks good. Feel free to add:
Acked-by: Jan Kara <[email protected]>
Honza
> ---
> fs/udf/inode.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index dce6ae9ae306c..0246b1b86fb91 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -182,11 +182,6 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
> }
> }
>
> -static int udf_writepage(struct page *page, struct writeback_control *wbc)
> -{
> - return block_write_full_page(page, udf_get_block, wbc);
> -}
> -
> static int udf_writepages(struct address_space *mapping,
> struct writeback_control *wbc)
> {
> @@ -239,12 +234,12 @@ const struct address_space_operations udf_aops = {
> .invalidate_folio = block_invalidate_folio,
> .read_folio = udf_read_folio,
> .readahead = udf_readahead,
> - .writepage = udf_writepage,
> .writepages = udf_writepages,
> .write_begin = udf_write_begin,
> .write_end = generic_write_end,
> .direct_IO = udf_direct_IO,
> .bmap = udf_bmap,
> + .migrate_folio = buffer_migrate_folio,
> };
>
> /*
> --
> 2.30.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 11/13/22 10:29AM, Christoph Hellwig wrote:
> ->writepage is a very inefficient method to write back data, and only
> used through write_cache_pages or a a fallback when no ->migrate_folio
> method is present.
>
> Set ->migrate_folio to the generic buffer_head based helper, and remove
> the ->writepage implementation.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Dave Kleikamp <[email protected]>
> ---
> fs/jfs/inode.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
> index d1ec920aa030a..8ac10e3960508 100644
> --- a/fs/jfs/inode.c
> +++ b/fs/jfs/inode.c
> @@ -264,11 +264,6 @@ int jfs_get_block(struct inode *ip, sector_t lblock,
> return rc;
> }
>
> -static int jfs_writepage(struct page *page, struct writeback_control *wbc)
> -{
> - return block_write_full_page(page, jfs_get_block, wbc);
> -}
> -
> static int jfs_writepages(struct address_space *mapping,
> struct writeback_control *wbc)
> {
> @@ -355,12 +350,12 @@ const struct address_space_operations jfs_aops = {
> .invalidate_folio = block_invalidate_folio,
> .read_folio = jfs_read_folio,
> .readahead = jfs_readahead,
> - .writepage = jfs_writepage,
> .writepages = jfs_writepages,
> .write_begin = jfs_write_begin,
> .write_end = jfs_write_end,
> .bmap = jfs_bmap,
> .direct_IO = jfs_direct_IO,
> + .migrate_folio = buffer_migrate_folio,
> };
>
> /*
On Sun, Nov 13, 2022 at 05:28:53PM +0100, Christoph Hellwig wrote:
> Hi all,
>
> The VM doesn't need or want ->writepage for writeback and is fine with
> just having ->writepages as long as ->migrate_folio is implemented.
>
> This series removes all ->writepage instances that use
> block_write_full_page directly and also have a plain mpage_writepages
> based ->writepages.
The series looks good from the MM side.
Acked-by: Johannes Weiner <[email protected]>
On Sun, Nov 13, 2022 at 05:29:01PM +0100, Christoph Hellwig wrote:
> ->writepage is a very inefficient method to write back data, and only
> used through write_cache_pages or a a fallback when no ->migrate_folio
> method is present.
>
> Set ->migrate_folio to the generic buffer_head based helper, and remove
> the ->writepage implementation.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
Looks good,
Acked-by: Bob Copeland <[email protected]>
--
Bob Copeland %% https://bobcopeland.com/
On Mon, Nov 14, 2022 at 11:49:27AM +0100, Jan Kara wrote:
> On Sun 13-11-22 17:28:55, Christoph Hellwig wrote:
> > ->writepage is a very inefficient method to write back data, and only
> > used through write_cache_pages or a a fallback when no ->migrate_folio
> > method is present.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
>
> Looks good! Feel free to add:
The testbot found a problem with this:
ext2_commit_chunk calls write_one_page for the IS_DIRSYNC case,
and write_one_page calls ->writepage.
So I think I need to drop this one for now (none of the other
file systems calls write_one_page). And then think what best
to do about write_one_page/write_one_folio. I suspect just
passing a writepage pointer to them might make most sense,
as they are only used by a few file systems, and the calling
convention with the locked page doesn't lend itself to using
->writepages.
On Wed 16-11-22 08:14:15, Christoph Hellwig wrote:
> On Mon, Nov 14, 2022 at 11:49:27AM +0100, Jan Kara wrote:
> > On Sun 13-11-22 17:28:55, Christoph Hellwig wrote:
> > > ->writepage is a very inefficient method to write back data, and only
> > > used through write_cache_pages or a a fallback when no ->migrate_folio
> > > method is present.
> > >
> > > Signed-off-by: Christoph Hellwig <[email protected]>
> >
> > Looks good! Feel free to add:
>
> The testbot found a problem with this:
>
> ext2_commit_chunk calls write_one_page for the IS_DIRSYNC case,
> and write_one_page calls ->writepage.
Right.
> So I think I need to drop this one for now (none of the other
> file systems calls write_one_page). And then think what best
> to do about write_one_page/write_one_folio. I suspect just
> passing a writepage pointer to them might make most sense,
> as they are only used by a few file systems, and the calling
> convention with the locked page doesn't lend itself to using
> ->writepages.
Looking at the code, IMO the write_one_page() looks somewhat premature
anyway in that place. AFAICS we could handle the writeout using
filemap_write_and_wait() if we moved it to somewhat later moment. So
something like attached patch (only basic testing only so far)?
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 22/11/13 05:28PM, Christoph Hellwig wrote:
> Hi all,
>
> The VM doesn't need or want ->writepage for writeback and is fine with
> just having ->writepages as long as ->migrate_folio is implemented.
Ok, so here is, (what I think) is the motivation for doing this.
Please correct me if this is incorrect...
1. writepage is mainly called from pageout, which happens as part of the memory
reclaim. Now IIUC from previous discussions [1][2][3], reclaims happens from
the tail end of the LRU list which could do an I/O of a single page while
an ongoing writeback was in progress of multiple pages. This disrupts the I/O
pattern to become more random in nature, compared to, if we would have let
writeback/flusher do it's job of writing back dirty pages.
Also many filesystems behave very differently within their ->writepage calls,
e.g. ext4 doesn't actually write in ->writepage for DELAYED blocks.
2. Now the other place from where ->writepage can be called from is, writeout()
function, which is a fallback function for migration (fallback_migrate_folio()).
fallback_migrate_folio() is called from move_to_new_folio() if ->migrate_folio
is not defined for the FS.
So what you are doing here is removing the ->writepage from address_space
operations of the filesystems which implements ->writepage using
block_write_full_page() (i.e. those who uses buffer_heads). This is done for
those FS who already have ->migrate_folio() implemented (hence no need of
->writepage).
...Now this is also a step towards reducing the callers from kernel which uses
buffer_heads.
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/
[3]: https://lore.kernel.org/all/[email protected]/
Is above a correct understanding?
>
> This series removes all ->writepage instances that use
> block_write_full_page directly and also have a plain mpage_writepages
> based ->writepages.
Ok.
>
> Diffstat:
> fs/exfat/inode.c | 9 ++-------
> fs/ext2/inode.c | 6 ------
> fs/fat/inode.c | 9 ++-------
> fs/hfs/inode.c | 2 +-
> fs/hfsplus/inode.c | 2 +-
> fs/hpfs/file.c | 9 ++-------
> fs/jfs/inode.c | 7 +------
> fs/omfs/file.c | 7 +------
> fs/udf/inode.c | 7 +------
> 9 files changed, 11 insertions(+), 47 deletions(-)
On Wed, Nov 16, 2022 at 07:20:40PM +0100, Jan Kara wrote:
> Looking at the code, IMO the write_one_page() looks somewhat premature
> anyway in that place. AFAICS we could handle the writeout using
> filemap_write_and_wait() if we moved it to somewhat later moment. So
> something like attached patch (only basic testing only so far)?
Yes, this looks sensible. Do you want to queue this one and the
ext2 and udf patches from this series if the testing works fine?
The same transformation should also be done for minix, sysfs and
ufs. And a bunch of the others are probaby similar as well.
Christoph Hellwig <[email protected]> wrote:
> The VM doesn't need or want ->writepage for writeback and is fine with
> just having ->writepages as long as ->migrate_folio is implemented.
Yay! :-)
David
Also ->writepage() is called with the page already locked, which is a problem
if you need to write out a number of surrounding pages with it.
David
On Wed 16-11-22 22:31:47, Christoph Hellwig wrote:
> On Wed, Nov 16, 2022 at 07:20:40PM +0100, Jan Kara wrote:
> > Looking at the code, IMO the write_one_page() looks somewhat premature
> > anyway in that place. AFAICS we could handle the writeout using
> > filemap_write_and_wait() if we moved it to somewhat later moment. So
> > something like attached patch (only basic testing only so far)?
>
> Yes, this looks sensible. Do you want to queue this one and the
> ext2 and udf patches from this series if the testing works fine?
OK, I'll take udf and ext2 patches through my tree.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Thu, Nov 17, 2022 at 12:09:00AM +0530, Ritesh Harjani (IBM) wrote:
> reclaim. Now IIUC from previous discussions [1][2][3], reclaims happens from
> the tail end of the LRU list which could do an I/O of a single page while
> an ongoing writeback was in progress of multiple pages. This disrupts the I/O
> pattern to become more random in nature, compared to, if we would have let
> writeback/flusher do it's job of writing back dirty pages.
Yes.
> Also many filesystems behave very differently within their ->writepage calls,
> e.g. ext4 doesn't actually write in ->writepage for DELAYED blocks.
I don't think it's many file systems. As far as I can tell only ext4
actually is significantly different.
> 2. Now the other place from where ->writepage can be called from is, writeout()
> function, which is a fallback function for migration (fallback_migrate_folio()).
> fallback_migrate_folio() is called from move_to_new_folio() if ->migrate_folio
> is not defined for the FS.
Also there is generic_writepages and folio_write_one/write_one_page.
> Is above a correct understanding?
Yes.
On Sun, Nov 13, 2022 at 05:28:57PM +0100, Christoph Hellwig wrote:
> ->writepage is a very inefficient method to write back data, and only
> used through write_cache_pages or a a fallback when no ->migrate_folio
> method is present.
>
> Set ->migrate_folio to the generic buffer_head based helper, and stop
> wiring up ->writepage for hfs_aops.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
Was there a reason you only did this for hfs_aops and not for
hfs_btree_aops? It feels like anything that just calls
block_write_full_page() in the writepage handler should be converted
to just calling mpage_writepages() in the writepages handler.
I have a few of those conversions done, but obviously they're in
filesystems that are basically untestable.
On Thu, Dec 14, 2023 at 07:01:25PM +0000, Matthew Wilcox wrote:
> Was there a reason you only did this for hfs_aops and not for
> hfs_btree_aops? It feels like anything that just calls
> block_write_full_page() in the writepage handler should be converted
> to just calling mpage_writepages() in the writepages handler.
> I have a few of those conversions done, but obviously they're in
> filesystems that are basically untestable.
Probably. I remember I had a good reason to skip, and the lack of
testability might have been it. Note that for hfsplus in particular
we should actually be able to test now that the port of the hfs
userspace has returned to distros. I haven't actually gotten to
see what the test baseline looks like, though.