linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [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).