Hi all, this series removes generic_writepages by open coding the current functionality in the three remaining callers. Besides removing some code the main benefit is that one of the few remaining ->writepage callers from outside the core page cache code go away. Note that testing has been a bit limited - ntfs3 does not seem to be supported by xfstests at all, and xfstests on ocfs2 is a complete shit show even for the base line. Diffstat: fs/jbd2/commit.c | 25 --------------------- fs/jbd2/journal.c | 1 fs/mpage.c | 8 ------ fs/ntfs3/inode.c | 33 +++++++++++++--------------- fs/ocfs2/journal.c | 16 +++++++++++++ include/linux/jbd2.h | 2 - include/linux/writeback.h | 2 - mm/page-writeback.c | 53 +++++++++++++--------------------------------- 8 files changed, 45 insertions(+), 95 deletions(-)
Christoph Hellwig
2022-Dec-29 16:10 UTC
[Ocfs2-devel] [PATCH 1/6] fs: remove an outdated comment on mpage_writepages
mpage_writepages doesn't do any of the page locking itself, so remove and outdated comment on the locking pattern there. Signed-off-by: Christoph Hellwig <hch at lst.de> --- fs/mpage.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fs/mpage.c b/fs/mpage.c index 0f8ae954a57903..910cfe8a60d2e4 100644 --- a/fs/mpage.c +++ b/fs/mpage.c @@ -641,14 +641,6 @@ static int __mpage_writepage(struct page *page, struct writeback_control *wbc, * * This is a library function, which implements the writepages() * address_space_operation. - * - * If a page is already under I/O, generic_writepages() skips it, even - * if it's dirty. This is desirable behaviour for memory-cleaning writeback, - * but it is INCORRECT for data-integrity system calls such as fsync(). fsync() - * and msync() need to guarantee that all the data which was dirty at the time - * the call was made get new I/O started against them. If wbc->sync_mode is - * WB_SYNC_ALL then we were called for data integrity and we must wait for - * existing IO to complete. */ int mpage_writepages(struct address_space *mapping, -- 2.35.1
Christoph Hellwig
2022-Dec-29 16:10 UTC
[Ocfs2-devel] [PATCH 2/6] ntfs3: stop using generic_writepages
Open code the resident inode handling in ntfs_writepages by directly
using write_cache_pages to prepare removing the ->writepage handler
in ntfs3.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
fs/ntfs3/inode.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 20b953871574b8..b6dad2da59501b 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -852,12 +852,29 @@ static int ntfs_writepage(struct page *page, struct
writeback_control *wbc)
return block_write_full_page(page, ntfs_get_block, wbc);
}
+static int ntfs_resident_writepage(struct page *page,
+ struct writeback_control *wbc, void *data)
+{
+ struct address_space *mapping = data;
+ struct ntfs_inode *ni = ntfs_i(mapping->host);
+ int ret;
+
+ ni_lock(ni);
+ ret = attr_data_write_resident(ni, page);
+ ni_unlock(ni);
+
+ if (ret != E_NTFS_NONRESIDENT)
+ unlock_page(page);
+ mapping_set_error(mapping, ret);
+ return ret;
+}
+
static int ntfs_writepages(struct address_space *mapping,
struct writeback_control *wbc)
{
- /* Redirect call to 'ntfs_writepage' for resident files. */
if (is_resident(ntfs_i(mapping->host)))
- return generic_writepages(mapping, wbc);
+ return write_cache_pages(mapping, wbc, ntfs_resident_writepage,
+ mapping);
return mpage_writepages(mapping, wbc, ntfs_get_block);
}
--
2.35.1
Christoph Hellwig
2022-Dec-29 16:10 UTC
[Ocfs2-devel] [PATCH 3/6] ntfs3: remove ->writepage
->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 <hch at lst.de>
---
fs/ntfs3/inode.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index b6dad2da59501b..6b50b6e3237876 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -832,26 +832,6 @@ int ntfs_set_size(struct inode *inode, u64 new_size)
return err;
}
-static int ntfs_writepage(struct page *page, struct writeback_control *wbc)
-{
- struct address_space *mapping = page->mapping;
- struct inode *inode = mapping->host;
- struct ntfs_inode *ni = ntfs_i(inode);
- int err;
-
- if (is_resident(ni)) {
- ni_lock(ni);
- err = attr_data_write_resident(ni, page);
- ni_unlock(ni);
- if (err != E_NTFS_NONRESIDENT) {
- unlock_page(page);
- return err;
- }
- }
-
- return block_write_full_page(page, ntfs_get_block, wbc);
-}
-
static int ntfs_resident_writepage(struct page *page,
struct writeback_control *wbc, void *data)
{
@@ -2083,13 +2063,13 @@ const struct inode_operations ntfs_link_inode_operations
= {
const struct address_space_operations ntfs_aops = {
.read_folio = ntfs_read_folio,
.readahead = ntfs_readahead,
- .writepage = ntfs_writepage,
.writepages = ntfs_writepages,
.write_begin = ntfs_write_begin,
.write_end = ntfs_write_end,
.direct_IO = ntfs_direct_IO,
.bmap = ntfs_bmap,
.dirty_folio = block_dirty_folio,
+ .migrate_folio = buffer_migrate_folio,
.invalidate_folio = block_invalidate_folio,
};
--
2.35.1
Christoph Hellwig
2022-Dec-29 16:10 UTC
[Ocfs2-devel] [PATCH 4/6] jbd2, ocfs2: move jbd2_journal_submit_inode_data_buffers to ocfs2
jbd2_journal_submit_inode_data_buffers is only used by ocfs2, so move
it there to prepare for removing generic_writepages.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
fs/jbd2/commit.c | 25 -------------------------
fs/jbd2/journal.c | 1 -
fs/ocfs2/journal.c | 16 +++++++++++++++-
include/linux/jbd2.h | 2 --
4 files changed, 15 insertions(+), 29 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 4810438b7856aa..aeee6b8a6da218 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -181,31 +181,6 @@ static int journal_wait_on_commit_record(journal_t
*journal,
return ret;
}
-/*
- * write the filemap data using writepage() address_space_operations.
- * We don't do block allocation here even for delalloc. We don't
- * use writepages() because with delayed allocation we may be doing
- * block allocation in writepages().
- */
-int jbd2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode)
-{
- struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
- struct writeback_control wbc = {
- .sync_mode = WB_SYNC_ALL,
- .nr_to_write = mapping->nrpages * 2,
- .range_start = jinode->i_dirty_start,
- .range_end = jinode->i_dirty_end,
- };
-
- /*
- * submit the inode data buffers. We use writepage
- * instead of writepages. Because writepages can do
- * block allocation with delalloc. We need to write
- * only allocated blocks here.
- */
- return generic_writepages(mapping, &wbc);
-}
-
/* Send all the data buffers related to an inode */
int jbd2_submit_inode_data(journal_t *journal, struct jbd2_inode *jinode)
{
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 2696f43e7239f8..d331f6ece20a28 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -89,7 +89,6 @@ EXPORT_SYMBOL(jbd2_journal_try_to_free_buffers);
EXPORT_SYMBOL(jbd2_journal_force_commit);
EXPORT_SYMBOL(jbd2_journal_inode_ranged_write);
EXPORT_SYMBOL(jbd2_journal_inode_ranged_wait);
-EXPORT_SYMBOL(jbd2_journal_submit_inode_data_buffers);
EXPORT_SYMBOL(jbd2_journal_finish_inode_data_buffers);
EXPORT_SYMBOL(jbd2_journal_init_jbd_inode);
EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 3fb98b4569a28b..59f612684c5178 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -15,6 +15,7 @@
#include <linux/time.h>
#include <linux/random.h>
#include <linux/delay.h>
+#include <linux/writeback.h>
#include <cluster/masklog.h>
@@ -841,6 +842,19 @@ int ocfs2_journal_alloc(struct ocfs2_super *osb)
return status;
}
+static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode)
+{
+ struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
+ struct writeback_control wbc = {
+ .sync_mode = WB_SYNC_ALL,
+ .nr_to_write = mapping->nrpages * 2,
+ .range_start = jinode->i_dirty_start,
+ .range_end = jinode->i_dirty_end,
+ };
+
+ return generic_writepages(mapping, &wbc);
+}
+
int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
{
int status = -1;
@@ -910,7 +924,7 @@ int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
journal->j_journal = j_journal;
journal->j_journal->j_submit_inode_data_buffers -
jbd2_journal_submit_inode_data_buffers;
+ ocfs2_journal_submit_inode_data_buffers;
journal->j_journal->j_finish_inode_data_buffers
jbd2_journal_finish_inode_data_buffers;
journal->j_inode = inode;
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 2170e0cc279d5c..5962072a4b19e6 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1570,8 +1570,6 @@ extern int jbd2_journal_inode_ranged_write(handle_t
*handle,
extern int jbd2_journal_inode_ranged_wait(handle_t *handle,
struct jbd2_inode *inode, loff_t start_byte,
loff_t length);
-extern int jbd2_journal_submit_inode_data_buffers(
- struct jbd2_inode *jinode);
extern int jbd2_journal_finish_inode_data_buffers(
struct jbd2_inode *jinode);
extern int jbd2_journal_begin_ordered_truncate(journal_t *journal,
--
2.35.1
Christoph Hellwig
2022-Dec-29 16:10 UTC
[Ocfs2-devel] [PATCH 5/6] ocfs2: use filemap_fdatawrite_wbc instead of generic_writepages
filemap_fdatawrite_wbc is a fairly thing wrapper around do_writepages, and the big difference there is support for cgroup writeback, which is not supported by ocfs2, and the potential to use ->writepages instead of ->writepage, which ocfs2 does not currently implement but eventually should. Signed-off-by: Christoph Hellwig <hch at lst.de> --- fs/ocfs2/journal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index 59f612684c5178..25d8072ccfce46 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -852,7 +852,7 @@ static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode) .range_end = jinode->i_dirty_end, }; - return generic_writepages(mapping, &wbc); + return filemap_fdatawrite_wbc(mapping, &wbc); } int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty) -- 2.35.1
Christoph Hellwig
2022-Dec-29 16:10 UTC
[Ocfs2-devel] [PATCH 6/6] mm: remove generic_writepages
Now that all external callers are gone, just fold it into do_writepages.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
include/linux/writeback.h | 2 --
mm/page-writeback.c | 53 +++++++++++----------------------------
2 files changed, 15 insertions(+), 40 deletions(-)
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 06f9291b6fd512..2554b71765e9d0 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -369,8 +369,6 @@ bool wb_over_bg_thresh(struct bdi_writeback *wb);
typedef int (*writepage_t)(struct page *page, struct writeback_control *wbc,
void *data);
-int generic_writepages(struct address_space *mapping,
- struct writeback_control *wbc);
void tag_pages_for_writeback(struct address_space *mapping,
pgoff_t start, pgoff_t end);
int write_cache_pages(struct address_space *mapping,
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index ad608ef2a24365..dfeeceebba0ae0 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2526,12 +2526,8 @@ int write_cache_pages(struct address_space *mapping,
}
EXPORT_SYMBOL(write_cache_pages);
-/*
- * Function used by generic_writepages to call the real writepage
- * function and set the mapping flags on error
- */
-static int __writepage(struct page *page, struct writeback_control *wbc,
- void *data)
+static int writepage_cb(struct page *page, struct writeback_control *wbc,
+ void *data)
{
struct address_space *mapping = data;
int ret = mapping->a_ops->writepage(page, wbc);
@@ -2539,34 +2535,6 @@ static int __writepage(struct page *page, struct
writeback_control *wbc,
return ret;
}
-/**
- * generic_writepages - walk the list of dirty pages of the given address space
and writepage() all of them.
- * @mapping: address space structure to write
- * @wbc: subtract the number of written pages from *@wbc->nr_to_write
- *
- * This is a library function, which implements the writepages()
- * address_space_operation.
- *
- * Return: %0 on success, negative error code otherwise
- */
-int generic_writepages(struct address_space *mapping,
- struct writeback_control *wbc)
-{
- struct blk_plug plug;
- int ret;
-
- /* deal with chardevs and other special file */
- if (!mapping->a_ops->writepage)
- return 0;
-
- blk_start_plug(&plug);
- ret = write_cache_pages(mapping, wbc, __writepage, mapping);
- blk_finish_plug(&plug);
- return ret;
-}
-
-EXPORT_SYMBOL(generic_writepages);
-
int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
{
int ret;
@@ -2577,11 +2545,20 @@ int do_writepages(struct address_space *mapping, struct
writeback_control *wbc)
wb = inode_to_wb_wbc(mapping->host, wbc);
wb_bandwidth_estimate_start(wb);
while (1) {
- if (mapping->a_ops->writepages)
+ if (mapping->a_ops->writepages) {
ret = mapping->a_ops->writepages(mapping, wbc);
- else
- ret = generic_writepages(mapping, wbc);
- if ((ret != -ENOMEM) || (wbc->sync_mode != WB_SYNC_ALL))
+ } else if (mapping->a_ops->writepage) {
+ struct blk_plug plug;
+
+ blk_start_plug(&plug);
+ ret = write_cache_pages(mapping, wbc, writepage_cb,
+ mapping);
+ blk_finish_plug(&plug);
+ } else {
+ /* deal with chardevs and other special files */
+ ret = 0;
+ }
+ if (ret != -ENOMEM || wbc->sync_mode != WB_SYNC_ALL)
break;
/*
--
2.35.1
Jan Kara
2023-Jan-02 12:15 UTC
[Ocfs2-devel] [PATCH 5/6] ocfs2: use filemap_fdatawrite_wbc instead of generic_writepages
On Thu 29-12-22 06:10:30, Christoph Hellwig wrote:> filemap_fdatawrite_wbc is a fairly thing wrapper around do_writepages, > and the big difference there is support for cgroup writeback, which > is not supported by ocfs2, and the potential to use ->writepages instead > of ->writepage, which ocfs2 does not currently implement but eventually > should. > > Signed-off-by: Christoph Hellwig <hch at lst.de>Looks good. Feel free to add: Reviewed-by: Jan Kara <jack at suse.cz> Honza> --- > fs/ocfs2/journal.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c > index 59f612684c5178..25d8072ccfce46 100644 > --- a/fs/ocfs2/journal.c > +++ b/fs/ocfs2/journal.c > @@ -852,7 +852,7 @@ static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode) > .range_end = jinode->i_dirty_end, > }; > > - return generic_writepages(mapping, &wbc); > + return filemap_fdatawrite_wbc(mapping, &wbc); > } > > int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty) > -- > 2.35.1 >-- Jan Kara <jack at suse.com> SUSE Labs, CR
On Thu 29-12-22 06:10:31, Christoph Hellwig wrote:> Now that all external callers are gone, just fold it into do_writepages. > > Signed-off-by: Christoph Hellwig <hch at lst.de>Looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack at suse.cz> Honza> --- > include/linux/writeback.h | 2 -- > mm/page-writeback.c | 53 +++++++++++---------------------------- > 2 files changed, 15 insertions(+), 40 deletions(-) > > diff --git a/include/linux/writeback.h b/include/linux/writeback.h > index 06f9291b6fd512..2554b71765e9d0 100644 > --- a/include/linux/writeback.h > +++ b/include/linux/writeback.h > @@ -369,8 +369,6 @@ bool wb_over_bg_thresh(struct bdi_writeback *wb); > typedef int (*writepage_t)(struct page *page, struct writeback_control *wbc, > void *data); > > -int generic_writepages(struct address_space *mapping, > - struct writeback_control *wbc); > void tag_pages_for_writeback(struct address_space *mapping, > pgoff_t start, pgoff_t end); > int write_cache_pages(struct address_space *mapping, > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index ad608ef2a24365..dfeeceebba0ae0 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2526,12 +2526,8 @@ int write_cache_pages(struct address_space *mapping, > } > EXPORT_SYMBOL(write_cache_pages); > > -/* > - * Function used by generic_writepages to call the real writepage > - * function and set the mapping flags on error > - */ > -static int __writepage(struct page *page, struct writeback_control *wbc, > - void *data) > +static int writepage_cb(struct page *page, struct writeback_control *wbc, > + void *data) > { > struct address_space *mapping = data; > int ret = mapping->a_ops->writepage(page, wbc); > @@ -2539,34 +2535,6 @@ static int __writepage(struct page *page, struct writeback_control *wbc, > return ret; > } > > -/** > - * generic_writepages - walk the list of dirty pages of the given address space and writepage() all of them. > - * @mapping: address space structure to write > - * @wbc: subtract the number of written pages from *@wbc->nr_to_write > - * > - * This is a library function, which implements the writepages() > - * address_space_operation. > - * > - * Return: %0 on success, negative error code otherwise > - */ > -int generic_writepages(struct address_space *mapping, > - struct writeback_control *wbc) > -{ > - struct blk_plug plug; > - int ret; > - > - /* deal with chardevs and other special file */ > - if (!mapping->a_ops->writepage) > - return 0; > - > - blk_start_plug(&plug); > - ret = write_cache_pages(mapping, wbc, __writepage, mapping); > - blk_finish_plug(&plug); > - return ret; > -} > - > -EXPORT_SYMBOL(generic_writepages); > - > int do_writepages(struct address_space *mapping, struct writeback_control *wbc) > { > int ret; > @@ -2577,11 +2545,20 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc) > wb = inode_to_wb_wbc(mapping->host, wbc); > wb_bandwidth_estimate_start(wb); > while (1) { > - if (mapping->a_ops->writepages) > + if (mapping->a_ops->writepages) { > ret = mapping->a_ops->writepages(mapping, wbc); > - else > - ret = generic_writepages(mapping, wbc); > - if ((ret != -ENOMEM) || (wbc->sync_mode != WB_SYNC_ALL)) > + } else if (mapping->a_ops->writepage) { > + struct blk_plug plug; > + > + blk_start_plug(&plug); > + ret = write_cache_pages(mapping, wbc, writepage_cb, > + mapping); > + blk_finish_plug(&plug); > + } else { > + /* deal with chardevs and other special files */ > + ret = 0; > + } > + if (ret != -ENOMEM || wbc->sync_mode != WB_SYNC_ALL) > break; > > /* > -- > 2.35.1 >-- Jan Kara <jack at suse.com> SUSE Labs, CR