* [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2
@ 2025-05-08 5:14 Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages() Christoph Hellwig
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
Hi all,
this almost entirely cleans up usage of AOP_WRITEPAGE_ACTIVATE in f2fs.
Changes since v1:
- pick up the bug fix from Chao as patch 1
- release the folio batch on early exit
- remove the dead for_reclaim handling
- keep the ability of the caller to exit early for the redity case
Diffstat:
fs/f2fs/checkpoint.c | 36 ++++++++----------------
fs/f2fs/compress.c | 3 --
fs/f2fs/data.c | 23 ++-------------
fs/f2fs/file.c | 1
fs/f2fs/node.c | 65 ++++++++++++++++++--------------------------
include/trace/events/f2fs.h | 5 ---
6 files changed, 46 insertions(+), 87 deletions(-)
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages()
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
@ 2025-05-08 5:14 ` Christoph Hellwig
2025-05-08 16:30 ` patchwork-bot+f2fs--- via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 2/6] f2fs: return bool from __f2fs_write_meta_folio Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, stable, linux-f2fs-devel
From: Chao Yu <chao@kernel.org>
If __write_node_folio() failed, it will return AOP_WRITEPAGE_ACTIVATE,
the incorrect return value may be passed to userspace in below path,
fix it.
- sync_filesystem
- sync_fs
- f2fs_issue_checkpoint
- block_operations
- f2fs_sync_node_pages
- __write_node_folio
: return AOP_WRITEPAGE_ACTIVATE
Cc: stable@vger.kernel.org
Reported-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/node.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index ec74eb9982a5..69308523c34e 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2092,10 +2092,14 @@ int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
ret = __write_node_folio(folio, false, &submitted,
wbc, do_balance, io_type, NULL);
- if (ret)
+ if (ret) {
folio_unlock(folio);
- else if (submitted)
+ folio_batch_release(&fbatch);
+ ret = -EIO;
+ goto out;
+ } else if (submitted) {
nwritten++;
+ }
if (--wbc->nr_to_write == 0)
break;
--
2.47.2
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [f2fs-dev] [PATCH 2/6] f2fs: return bool from __f2fs_write_meta_folio
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages() Christoph Hellwig
@ 2025-05-08 5:14 ` Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 3/6] f2fs: remove wbc->for_reclaim handling Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
__f2fs_write_meta_folio can only return 0 or AOP_WRITEPAGE_ACTIVATE.
As part of phasing out AOP_WRITEPAGE_ACTIVATE, switch to a bool return
instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/checkpoint.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index e42ed62fa45c..595d6e87aa2f 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -340,7 +340,7 @@ void f2fs_ra_meta_pages_cond(struct f2fs_sb_info *sbi, pgoff_t index,
f2fs_ra_meta_pages(sbi, index, ra_blocks, META_POR, true);
}
-static int __f2fs_write_meta_folio(struct folio *folio,
+static bool __f2fs_write_meta_folio(struct folio *folio,
struct writeback_control *wbc,
enum iostat_type io_type)
{
@@ -353,7 +353,7 @@ static int __f2fs_write_meta_folio(struct folio *folio,
folio_clear_uptodate(folio);
dec_page_count(sbi, F2FS_DIRTY_META);
folio_unlock(folio);
- return 0;
+ return true;
}
goto redirty_out;
}
@@ -373,11 +373,11 @@ static int __f2fs_write_meta_folio(struct folio *folio,
if (unlikely(f2fs_cp_error(sbi)))
f2fs_submit_merged_write(sbi, META);
- return 0;
+ return true;
redirty_out:
folio_redirty_for_writepage(wbc, folio);
- return AOP_WRITEPAGE_ACTIVATE;
+ return false;
}
static int f2fs_write_meta_pages(struct address_space *mapping,
@@ -461,7 +461,7 @@ long f2fs_sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
if (!folio_clear_dirty_for_io(folio))
goto continue_unlock;
- if (__f2fs_write_meta_folio(folio, &wbc,
+ if (!__f2fs_write_meta_folio(folio, &wbc,
io_type)) {
folio_unlock(folio);
break;
@@ -1409,7 +1409,6 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
* f2fs_sync_meta_pages are combined in this function.
*/
struct folio *folio = f2fs_grab_meta_folio(sbi, blk_addr);
- int err;
memcpy(folio_address(folio), src, PAGE_SIZE);
@@ -1418,13 +1417,14 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
f2fs_bug_on(sbi, 1);
/* writeout cp pack 2 page */
- err = __f2fs_write_meta_folio(folio, &wbc, FS_CP_META_IO);
- if (unlikely(err && f2fs_cp_error(sbi))) {
- f2fs_folio_put(folio, true);
- return;
+ if (unlikely(!__f2fs_write_meta_folio(folio, &wbc, FS_CP_META_IO))) {
+ if (f2fs_cp_error(sbi)) {
+ f2fs_folio_put(folio, true);
+ return;
+ }
+ f2fs_bug_on(sbi, true);
}
- f2fs_bug_on(sbi, err);
f2fs_folio_put(folio, false);
/* submit checkpoint (with barrier if NOBARRIER is not set) */
--
2.47.2
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [f2fs-dev] [PATCH 3/6] f2fs: remove wbc->for_reclaim handling
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages() Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 2/6] f2fs: return bool from __f2fs_write_meta_folio Christoph Hellwig
@ 2025-05-08 5:14 ` Christoph Hellwig
2025-05-08 9:46 ` Chao Yu via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 4/6] f2fs: always unlock the page in f2fs_write_single_data_page Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
Since commits 7ff0104a8052 ("f2fs: Remove f2fs_write_node_page()") and
3b47398d9861 ("f2fs: Remove f2fs_write_meta_page()'), f2fs can't be
called from reclaim context any more. Remove all code keyed of the
wbc->for_rename flag, which is now only set for writing out swap or
shmem pages inside the swap code, but never passed to file systems.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/f2fs/checkpoint.c | 14 ++------------
fs/f2fs/data.c | 17 ++---------------
fs/f2fs/file.c | 1 -
fs/f2fs/node.c | 13 +------------
include/trace/events/f2fs.h | 5 +----
5 files changed, 6 insertions(+), 44 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 595d6e87aa2f..e7907858eb70 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -359,15 +359,10 @@ static bool __f2fs_write_meta_folio(struct folio *folio,
}
if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
goto redirty_out;
- if (wbc->for_reclaim && folio->index < GET_SUM_BLOCK(sbi, 0))
- goto redirty_out;
f2fs_do_write_meta_page(sbi, folio, io_type);
dec_page_count(sbi, F2FS_DIRTY_META);
- if (wbc->for_reclaim)
- f2fs_submit_merged_write_cond(sbi, NULL, &folio->page, 0, META);
-
folio_unlock(folio);
if (unlikely(f2fs_cp_error(sbi)))
@@ -420,9 +415,7 @@ long f2fs_sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
struct folio_batch fbatch;
long nwritten = 0;
int nr_folios;
- struct writeback_control wbc = {
- .for_reclaim = 0,
- };
+ struct writeback_control wbc = {};
struct blk_plug plug;
folio_batch_init(&fbatch);
@@ -1215,7 +1208,6 @@ static int block_operations(struct f2fs_sb_info *sbi)
struct writeback_control wbc = {
.sync_mode = WB_SYNC_ALL,
.nr_to_write = LONG_MAX,
- .for_reclaim = 0,
};
int err = 0, cnt = 0;
@@ -1399,9 +1391,7 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc)
static void commit_checkpoint(struct f2fs_sb_info *sbi,
void *src, block_t blk_addr)
{
- struct writeback_control wbc = {
- .for_reclaim = 0,
- };
+ struct writeback_control wbc = {};
/*
* filemap_get_folios_tag and folio_lock again will take
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a1eb740a2b5c..160c7b39d967 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2856,13 +2856,7 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
goto done;
}
- if (!wbc->for_reclaim)
- need_balance_fs = true;
- else if (has_not_enough_free_secs(sbi, 0, 0))
- goto redirty_out;
- else
- set_inode_flag(inode, FI_HOT_DATA);
-
+ need_balance_fs = true;
err = -EAGAIN;
if (f2fs_has_inline_data(inode)) {
err = f2fs_write_inline_data(inode, folio);
@@ -2898,13 +2892,6 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
folio_clear_uptodate(folio);
clear_page_private_gcing(page);
}
-
- if (wbc->for_reclaim) {
- f2fs_submit_merged_write_cond(sbi, NULL, page, 0, DATA);
- clear_inode_flag(inode, FI_HOT_DATA);
- f2fs_remove_dirty_inode(inode);
- submitted = NULL;
- }
folio_unlock(folio);
if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode) &&
!F2FS_I(inode)->wb_task && allow_balance)
@@ -2930,7 +2917,7 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
* file_write_and_wait_range() will see EIO error, which is critical
* to return value of fsync() followed by atomic_write failure to user.
*/
- if (!err || wbc->for_reclaim)
+ if (!err)
return AOP_WRITEPAGE_ACTIVATE;
folio_unlock(folio);
return err;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 93d85defac53..6bd3de64f2a8 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -261,7 +261,6 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
struct writeback_control wbc = {
.sync_mode = WB_SYNC_ALL,
.nr_to_write = LONG_MAX,
- .for_reclaim = 0,
};
unsigned int seq_id = 0;
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 69308523c34e..f6e98c9fac95 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1699,12 +1699,7 @@ static int __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
if (f2fs_get_node_info(sbi, nid, &ni, !do_balance))
goto redirty_out;
- if (wbc->for_reclaim) {
- if (!f2fs_down_read_trylock(&sbi->node_write))
- goto redirty_out;
- } else {
- f2fs_down_read(&sbi->node_write);
- }
+ f2fs_down_read(&sbi->node_write);
/* This page is already truncated */
if (unlikely(ni.blk_addr == NULL_ADDR)) {
@@ -1740,11 +1735,6 @@ static int __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
dec_page_count(sbi, F2FS_DIRTY_NODES);
f2fs_up_read(&sbi->node_write);
- if (wbc->for_reclaim) {
- f2fs_submit_merged_write_cond(sbi, NULL, &folio->page, 0, NODE);
- submitted = NULL;
- }
-
folio_unlock(folio);
if (unlikely(f2fs_cp_error(sbi))) {
@@ -1771,7 +1761,6 @@ int f2fs_move_node_folio(struct folio *node_folio, int gc_type)
struct writeback_control wbc = {
.sync_mode = WB_SYNC_ALL,
.nr_to_write = 1,
- .for_reclaim = 0,
};
f2fs_folio_wait_writeback(node_folio, NODE, true, true);
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index eb3b2f1326b1..edbbd869078f 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -1472,7 +1472,6 @@ TRACE_EVENT(f2fs_writepages,
__field(char, for_kupdate)
__field(char, for_background)
__field(char, tagged_writepages)
- __field(char, for_reclaim)
__field(char, range_cyclic)
__field(char, for_sync)
),
@@ -1491,14 +1490,13 @@ TRACE_EVENT(f2fs_writepages,
__entry->for_kupdate = wbc->for_kupdate;
__entry->for_background = wbc->for_background;
__entry->tagged_writepages = wbc->tagged_writepages;
- __entry->for_reclaim = wbc->for_reclaim;
__entry->range_cyclic = wbc->range_cyclic;
__entry->for_sync = wbc->for_sync;
),
TP_printk("dev = (%d,%d), ino = %lu, %s, %s, nr_to_write %ld, "
"skipped %ld, start %lld, end %lld, wb_idx %lu, sync_mode %d, "
- "kupdate %u background %u tagged %u reclaim %u cyclic %u sync %u",
+ "kupdate %u background %u tagged %u cyclic %u sync %u",
show_dev_ino(__entry),
show_block_type(__entry->type),
show_file_type(__entry->dir),
@@ -1511,7 +1509,6 @@ TRACE_EVENT(f2fs_writepages,
__entry->for_kupdate,
__entry->for_background,
__entry->tagged_writepages,
- __entry->for_reclaim,
__entry->range_cyclic,
__entry->for_sync)
);
--
2.47.2
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [f2fs-dev] [PATCH 4/6] f2fs: always unlock the page in f2fs_write_single_data_page
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
` (2 preceding siblings ...)
2025-05-08 5:14 ` [f2fs-dev] [PATCH 3/6] f2fs: remove wbc->for_reclaim handling Christoph Hellwig
@ 2025-05-08 5:14 ` Christoph Hellwig
2025-05-08 9:48 ` Chao Yu via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 6/6] f2fs: return bool from __write_node_folio Christoph Hellwig
5 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
Consolidate the code to unlock the page in f2fs_write_single_data_page
instead of leaving it to the callers for the AOP_WRITEPAGE_ACTIVATE case.
Replace AOP_WRITEPAGE_ACTIVATE with a positive return of 1 as this case
now doesn't match the historic ->writepage special return code that is
on it's way out now that ->writepage has been removed.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/f2fs/compress.c | 3 +--
fs/f2fs/data.c | 8 +++-----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index e016b0f96313..1e62fdffda07 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1565,8 +1565,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
NULL, NULL, wbc, io_type,
compr_blocks, false);
if (ret) {
- if (ret == AOP_WRITEPAGE_ACTIVATE) {
- folio_unlock(folio);
+ if (ret == 1) {
ret = 0;
} else if (ret == -EAGAIN) {
ret = 0;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 160c7b39d967..8d8018083c31 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2917,9 +2917,9 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
* file_write_and_wait_range() will see EIO error, which is critical
* to return value of fsync() followed by atomic_write failure to user.
*/
- if (!err)
- return AOP_WRITEPAGE_ACTIVATE;
folio_unlock(folio);
+ if (!err)
+ return 1;
return err;
}
@@ -3133,8 +3133,6 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
ret = f2fs_write_single_data_page(folio,
&submitted, &bio, &last_block,
wbc, io_type, 0, true);
- if (ret == AOP_WRITEPAGE_ACTIVATE)
- folio_unlock(folio);
#ifdef CONFIG_F2FS_FS_COMPRESSION
result:
#endif
@@ -3146,7 +3144,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
* keep nr_to_write, since vfs uses this to
* get # of written pages.
*/
- if (ret == AOP_WRITEPAGE_ACTIVATE) {
+ if (ret == 1) {
ret = 0;
goto next;
} else if (ret == -EAGAIN) {
--
2.47.2
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [f2fs-dev] [PATCH 5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
` (3 preceding siblings ...)
2025-05-08 5:14 ` [f2fs-dev] [PATCH 4/6] f2fs: always unlock the page in f2fs_write_single_data_page Christoph Hellwig
@ 2025-05-08 5:14 ` Christoph Hellwig
2025-05-08 9:53 ` Chao Yu via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 6/6] f2fs: return bool from __write_node_folio Christoph Hellwig
5 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
Always assign ret where the error happens, and jump to out instead
of multiple loop exit conditions to prepare for changes in the
__write_node_folio calling convention.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/f2fs/node.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index f6e98c9fac95..cbc7e9997b74 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1871,31 +1871,30 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
if (!folio_clear_dirty_for_io(folio))
goto continue_unlock;
- ret = __write_node_folio(folio, atomic &&
+ if (__write_node_folio(folio, atomic &&
folio == last_folio,
&submitted, wbc, true,
- FS_NODE_IO, seq_id);
- if (ret) {
+ FS_NODE_IO, seq_id)) {
folio_unlock(folio);
f2fs_folio_put(last_folio, false);
- break;
- } else if (submitted) {
- nwritten++;
+ folio_batch_release(&fbatch);
+ ret = -EIO;
+ goto out;
}
+ if (submitted)
+ nwritten++;
if (folio == last_folio) {
f2fs_folio_put(folio, false);
+ folio_batch_release(&fbatch);
marked = true;
- break;
+ goto out;
}
}
folio_batch_release(&fbatch);
cond_resched();
-
- if (ret || marked)
- break;
}
- if (!ret && atomic && !marked) {
+ if (atomic && !marked) {
f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
ino, last_folio->index);
folio_lock(last_folio);
@@ -1907,7 +1906,7 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
out:
if (nwritten)
f2fs_submit_merged_write_cond(sbi, NULL, NULL, ino, NODE);
- return ret ? -EIO : 0;
+ return ret;
}
static int f2fs_match_ino(struct inode *inode, unsigned long ino, void *data)
--
2.47.2
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [f2fs-dev] [PATCH 6/6] f2fs: return bool from __write_node_folio
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
` (4 preceding siblings ...)
2025-05-08 5:14 ` [f2fs-dev] [PATCH 5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages Christoph Hellwig
@ 2025-05-08 5:14 ` Christoph Hellwig
5 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2025-05-08 5:14 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
__write_node_folio can only return 0 or AOP_WRITEPAGE_ACTIVATE.
As part of phasing out AOP_WRITEPAGE_ACTIVATE, switch to a bool return
instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/node.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index cbc7e9997b74..3f6b8037d25f 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1651,7 +1651,7 @@ static struct folio *last_fsync_dnode(struct f2fs_sb_info *sbi, nid_t ino)
return last_folio;
}
-static int __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
+static bool __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
struct writeback_control *wbc, bool do_balance,
enum iostat_type io_type, unsigned int *seq_id)
{
@@ -1681,7 +1681,7 @@ static int __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
folio_clear_uptodate(folio);
dec_page_count(sbi, F2FS_DIRTY_NODES);
folio_unlock(folio);
- return 0;
+ return true;
}
if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
@@ -1707,7 +1707,7 @@ static int __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
dec_page_count(sbi, F2FS_DIRTY_NODES);
f2fs_up_read(&sbi->node_write);
folio_unlock(folio);
- return 0;
+ return true;
}
if (__is_valid_data_blkaddr(ni.blk_addr) &&
@@ -1746,11 +1746,12 @@ static int __write_node_folio(struct folio *folio, bool atomic, bool *submitted,
if (do_balance)
f2fs_balance_fs(sbi, false);
- return 0;
+ return true;
redirty_out:
folio_redirty_for_writepage(wbc, folio);
- return AOP_WRITEPAGE_ACTIVATE;
+ folio_unlock(folio);
+ return false;
}
int f2fs_move_node_folio(struct folio *node_folio, int gc_type)
@@ -1772,11 +1773,9 @@ int f2fs_move_node_folio(struct folio *node_folio, int gc_type)
goto out_page;
}
- if (__write_node_folio(node_folio, false, NULL,
- &wbc, false, FS_GC_NODE_IO, NULL)) {
+ if (!__write_node_folio(node_folio, false, NULL,
+ &wbc, false, FS_GC_NODE_IO, NULL))
err = -EAGAIN;
- folio_unlock(node_folio);
- }
goto release_page;
} else {
/* set page dirty and write it */
@@ -1871,11 +1870,10 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
if (!folio_clear_dirty_for_io(folio))
goto continue_unlock;
- if (__write_node_folio(folio, atomic &&
+ if (!__write_node_folio(folio, atomic &&
folio == last_folio,
&submitted, wbc, true,
FS_NODE_IO, seq_id)) {
- folio_unlock(folio);
f2fs_folio_put(last_folio, false);
folio_batch_release(&fbatch);
ret = -EIO;
@@ -2078,16 +2076,15 @@ int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
set_fsync_mark(&folio->page, 0);
set_dentry_mark(&folio->page, 0);
- ret = __write_node_folio(folio, false, &submitted,
- wbc, do_balance, io_type, NULL);
- if (ret) {
+ if (!__write_node_folio(folio, false, &submitted,
+ wbc, do_balance, io_type, NULL)) {
folio_unlock(folio);
folio_batch_release(&fbatch);
ret = -EIO;
goto out;
- } else if (submitted) {
- nwritten++;
}
+ if (submitted)
+ nwritten++;
if (--wbc->nr_to_write == 0)
break;
--
2.47.2
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [f2fs-dev] [PATCH 3/6] f2fs: remove wbc->for_reclaim handling
2025-05-08 5:14 ` [f2fs-dev] [PATCH 3/6] f2fs: remove wbc->for_reclaim handling Christoph Hellwig
@ 2025-05-08 9:46 ` Chao Yu via Linux-f2fs-devel
0 siblings, 0 replies; 11+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-05-08 9:46 UTC (permalink / raw)
To: Christoph Hellwig, Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 5/8/25 13:14, Christoph Hellwig wrote:
> Since commits 7ff0104a8052 ("f2fs: Remove f2fs_write_node_page()") and
> 3b47398d9861 ("f2fs: Remove f2fs_write_meta_page()'), f2fs can't be
> called from reclaim context any more. Remove all code keyed of the
> wbc->for_rename flag, which is now only set for writing out swap or
s/for_rename/for_reclaim
I guess Jaegeuk can fix this during merge.
> shmem pages inside the swap code, but never passed to file systems.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [f2fs-dev] [PATCH 4/6] f2fs: always unlock the page in f2fs_write_single_data_page
2025-05-08 5:14 ` [f2fs-dev] [PATCH 4/6] f2fs: always unlock the page in f2fs_write_single_data_page Christoph Hellwig
@ 2025-05-08 9:48 ` Chao Yu via Linux-f2fs-devel
0 siblings, 0 replies; 11+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-05-08 9:48 UTC (permalink / raw)
To: Christoph Hellwig, Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 5/8/25 13:14, Christoph Hellwig wrote:
> Consolidate the code to unlock the page in f2fs_write_single_data_page
> instead of leaving it to the callers for the AOP_WRITEPAGE_ACTIVATE case.
> Replace AOP_WRITEPAGE_ACTIVATE with a positive return of 1 as this case
> now doesn't match the historic ->writepage special return code that is
> on it's way out now that ->writepage has been removed.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [f2fs-dev] [PATCH 5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages
2025-05-08 5:14 ` [f2fs-dev] [PATCH 5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages Christoph Hellwig
@ 2025-05-08 9:53 ` Chao Yu via Linux-f2fs-devel
0 siblings, 0 replies; 11+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-05-08 9:53 UTC (permalink / raw)
To: Christoph Hellwig, Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 5/8/25 13:14, Christoph Hellwig wrote:
> Always assign ret where the error happens, and jump to out instead
> of multiple loop exit conditions to prepare for changes in the
> __write_node_folio calling convention.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages()
2025-05-08 5:14 ` [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages() Christoph Hellwig
@ 2025-05-08 16:30 ` patchwork-bot+f2fs--- via Linux-f2fs-devel
0 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+f2fs--- via Linux-f2fs-devel @ 2025-05-08 16:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: jaegeuk, linux-kernel, stable, linux-f2fs-devel
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Thu, 8 May 2025 07:14:27 +0200 you wrote:
> From: Chao Yu <chao@kernel.org>
>
> If __write_node_folio() failed, it will return AOP_WRITEPAGE_ACTIVATE,
> the incorrect return value may be passed to userspace in below path,
> fix it.
>
> - sync_filesystem
> - sync_fs
> - f2fs_issue_checkpoint
> - block_operations
> - f2fs_sync_node_pages
> - __write_node_folio
> : return AOP_WRITEPAGE_ACTIVATE
>
> [...]
Here is the summary with links:
- [f2fs-dev,1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages()
https://git.kernel.org/jaegeuk/f2fs/c/43ba56a043b1
- [f2fs-dev,2/6] f2fs: return bool from __f2fs_write_meta_folio
https://git.kernel.org/jaegeuk/f2fs/c/39122e454419
- [f2fs-dev,3/6] f2fs: remove wbc->for_reclaim handling
https://git.kernel.org/jaegeuk/f2fs/c/402dd9f02ce4
- [f2fs-dev,4/6] f2fs: always unlock the page in f2fs_write_single_data_page
https://git.kernel.org/jaegeuk/f2fs/c/84c5d16711a3
- [f2fs-dev,5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages
https://git.kernel.org/jaegeuk/f2fs/c/0638f28b3062
- [f2fs-dev,6/6] f2fs: return bool from __write_node_folio
https://git.kernel.org/jaegeuk/f2fs/c/80f31d2a7e5f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-05-08 16:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-08 5:14 [f2fs-dev] cleanup AOP_WRITEPAGE_ACTIVATE use in f2fs v2 Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 1/6] f2fs: fix to return correct error number in f2fs_sync_node_pages() Christoph Hellwig
2025-05-08 16:30 ` patchwork-bot+f2fs--- via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 2/6] f2fs: return bool from __f2fs_write_meta_folio Christoph Hellwig
2025-05-08 5:14 ` [f2fs-dev] [PATCH 3/6] f2fs: remove wbc->for_reclaim handling Christoph Hellwig
2025-05-08 9:46 ` Chao Yu via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 4/6] f2fs: always unlock the page in f2fs_write_single_data_page Christoph Hellwig
2025-05-08 9:48 ` Chao Yu via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 5/6] f2fs: simplify return value handling in f2fs_fsync_node_pages Christoph Hellwig
2025-05-08 9:53 ` Chao Yu via Linux-f2fs-devel
2025-05-08 5:14 ` [f2fs-dev] [PATCH 6/6] f2fs: return bool from __write_node_folio Christoph Hellwig
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).