2023-12-15 20:08:05

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 00/14] Clean up the writeback paths

I don't think any of this conflicts with the writeback refactoring that
Christoph has kindly taken over from me, although we might want to redo
patch 13 on that infrastructure rather than using write_cache_pages().
That can be a later addition.

Most of these patches verge on the trivial, converting filesystems that
just use block_write_full_page() to use mpage_writepages(). But as we
saw with Christoph's earlier patchset, there can be some "interesting"
gotchas, and I clearly haven't tested the majority of filesystems I've
touched here.

Patches 3 & 4 get rid of a lot of stack usage on architectures with
larger page sizes; 1024 bytes on 64-bit systems with 64KiB pages.
It starts to open the door to larger folio sizes on all architectures,
but it's certainly not enough yet.

Patch 14 is kind of trivial, but it's nice to get that simplification in.

Matthew Wilcox (Oracle) (14):
fs: Remove clean_page_buffers()
fs: Convert clean_buffers() to take a folio
fs: Reduce stack usage in __mpage_writepage
fs: Reduce stack usage in do_mpage_readpage
adfs: Remove writepage implementation
bfs: Remove writepage implementation
hfs: Really remove hfs_writepage
hfsplus: Really remove hfsplus_writepage
minix: Remove writepage implementation
ocfs2: Remove writepage implementation
sysv: Remove writepage implementation
ufs: Remove writepage implementation
fs: Convert block_write_full_page to block_write_full_folio
fs: Remove the bh_end_io argument from __block_write_full_folio

block/fops.c | 21 +++++++++++--
fs/adfs/inode.c | 11 ++++---
fs/bfs/file.c | 9 ++++--
fs/buffer.c | 36 ++++++++++-----------
fs/ext4/page-io.c | 2 +-
fs/gfs2/aops.c | 6 ++--
fs/hfs/inode.c | 8 ++---
fs/hfsplus/inode.c | 8 ++---
fs/minix/inode.c | 9 ++++--
fs/mpage.c | 62 +++++++++++++++++--------------------
fs/ntfs/aops.c | 4 +--
fs/ocfs2/alloc.c | 2 +-
fs/ocfs2/aops.c | 15 ++++-----
fs/ocfs2/file.c | 2 +-
fs/ocfs2/ocfs2_trace.h | 2 --
fs/sysv/itree.c | 9 ++++--
fs/ufs/inode.c | 11 ++++---
include/linux/buffer_head.h | 9 ++----
18 files changed, 115 insertions(+), 111 deletions(-)

--
2.42.0



2023-12-15 20:12:09

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 06/14] bfs: Remove writepage implementation

If the filesystem implements migrate_folio and writepages, there is
no need for a writepage implementation.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/bfs/file.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index adc2230079c6..a778411574a9 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -11,6 +11,7 @@
*/

#include <linux/fs.h>
+#include <linux/mpage.h>
#include <linux/buffer_head.h>
#include "bfs.h"

@@ -150,9 +151,10 @@ static int bfs_get_block(struct inode *inode, sector_t block,
return err;
}

-static int bfs_writepage(struct page *page, struct writeback_control *wbc)
+static int bfs_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
{
- return block_write_full_page(page, bfs_get_block, wbc);
+ return mpage_writepages(mapping, wbc, bfs_get_block);
}

static int bfs_read_folio(struct file *file, struct folio *folio)
@@ -190,9 +192,10 @@ const struct address_space_operations bfs_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = bfs_read_folio,
- .writepage = bfs_writepage,
+ .writepages = bfs_writepages,
.write_begin = bfs_write_begin,
.write_end = generic_write_end,
+ .migrate_folio = buffer_migrate_folio,
.bmap = bfs_bmap,
};

--
2.42.0


2023-12-15 20:14:20

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 02/14] fs: Convert clean_buffers() to take a folio

The only caller already has a folio, so pass it in and use it throughout.
Saves two calls to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/mpage.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/mpage.c b/fs/mpage.c
index 63bf99856024..630f4a7c7d03 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -430,13 +430,13 @@ struct mpage_data {
* We have our BIO, so we can now mark the buffers clean. Make
* sure to only clean buffers which we know we'll be writing.
*/
-static void clean_buffers(struct page *page, unsigned first_unmapped)
+static void clean_buffers(struct folio *folio, unsigned first_unmapped)
{
unsigned buffer_counter = 0;
- struct buffer_head *bh, *head;
- if (!page_has_buffers(page))
+ struct buffer_head *bh, *head = folio_buffers(folio);
+
+ if (!head)
return;
- head = page_buffers(page);
bh = head;

do {
@@ -451,8 +451,8 @@ static void clean_buffers(struct page *page, unsigned first_unmapped)
* read_folio would fail to serialize with the bh and it would read from
* disk before we reach the platter.
*/
- if (buffer_heads_over_limit && PageUptodate(page))
- try_to_free_buffers(page_folio(page));
+ if (buffer_heads_over_limit && folio_test_uptodate(folio))
+ try_to_free_buffers(folio);
}

static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,
@@ -615,7 +615,7 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,
goto alloc_new;
}

- clean_buffers(&folio->page, first_unmapped);
+ clean_buffers(folio, first_unmapped);

BUG_ON(folio_test_writeback(folio));
folio_start_writeback(folio);
--
2.42.0


2023-12-15 20:14:29

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 05/14] adfs: Remove writepage implementation

If the filesystem implements migrate_folio and writepages, there is
no need for a writepage implementation.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/adfs/inode.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/adfs/inode.c b/fs/adfs/inode.c
index 3081edb09e46..a183e213a4a5 100644
--- a/fs/adfs/inode.c
+++ b/fs/adfs/inode.c
@@ -5,6 +5,7 @@
* Copyright (C) 1997-1999 Russell King
*/
#include <linux/buffer_head.h>
+#include <linux/mpage.h>
#include <linux/writeback.h>
#include "adfs.h"

@@ -33,9 +34,10 @@ adfs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh,
return 0;
}

-static int adfs_writepage(struct page *page, struct writeback_control *wbc)
+static int adfs_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
{
- return block_write_full_page(page, adfs_get_block, wbc);
+ return mpage_writepages(mapping, wbc, adfs_get_block);
}

static int adfs_read_folio(struct file *file, struct folio *folio)
@@ -76,10 +78,11 @@ static const struct address_space_operations adfs_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = adfs_read_folio,
- .writepage = adfs_writepage,
+ .writepages = adfs_writepages,
.write_begin = adfs_write_begin,
.write_end = generic_write_end,
- .bmap = _adfs_bmap
+ .migrate_folio = buffer_migrate_folio,
+ .bmap = _adfs_bmap,
};

/*
--
2.42.0


2023-12-16 04:28:14

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 02/14] fs: Convert clean_buffers() to take a folio

On Fri, Dec 15, 2023 at 08:02:33PM +0000, Matthew Wilcox (Oracle) wrote:
> The only caller already has a folio, so pass it in and use it throughout.
> Saves two calls to compound_head().

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2023-12-16 04:32:12

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 05/14] adfs: Remove writepage implementation

On Fri, Dec 15, 2023 at 08:02:36PM +0000, Matthew Wilcox (Oracle) wrote:
> If the filesystem implements migrate_folio and writepages, there is
> no need for a writepage implementation.

Looks good to me:

Reviewed-by: Christoph Hellwig <[email protected]>

(with the usual caveat for basically untestable otheros fses that'll
also apply to various later patches)

2023-12-16 20:51:36

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 00/14] Clean up the writeback paths

On 12/15/23 1:02 PM, Matthew Wilcox (Oracle) wrote:
> I don't think any of this conflicts with the writeback refactoring that
> Christoph has kindly taken over from me, although we might want to redo
> patch 13 on that infrastructure rather than using write_cache_pages().
> That can be a later addition.
>
> Most of these patches verge on the trivial, converting filesystems that
> just use block_write_full_page() to use mpage_writepages(). But as we
> saw with Christoph's earlier patchset, there can be some "interesting"
> gotchas, and I clearly haven't tested the majority of filesystems I've
> touched here.
>
> Patches 3 & 4 get rid of a lot of stack usage on architectures with
> larger page sizes; 1024 bytes on 64-bit systems with 64KiB pages.
> It starts to open the door to larger folio sizes on all architectures,
> but it's certainly not enough yet.
>
> Patch 14 is kind of trivial, but it's nice to get that simplification in.

Series looks good to me:

Reviewed-by: Jens Axboe <[email protected]>

--
Jens Axboe



2023-12-17 16:48:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 06/14] bfs: Remove writepage implementation

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>