Re: [f2fs-dev] [PATCH 05/14] f2fs: support large folio writeback

[Nanzhe Zhao]

On Thu, 27 Aug 2026 15:39:19 -0700, Daeho Jeong wrote:
> If f2fs_write_single_data_folio() fails on subpage 1, jumping
> to `out` unconditionally clears the dirty bitmap for the ENTIRE folio
> (0 to folio_size),
> and decrements the inode dirty pages.
>
> The unwritten dirty subpages lose their dirty tracking and will be
> discarded cleanly
> under memory pressure, causing silent data loss.
>
> Fix: Only clear dirty bits for successfully submitted subranges (`0,
> pos`), and redirty
> the folio if unwritten dirty subpages remain when an error occurs.

Thanks for the review. Agreed.
My change hunk may like this:

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
@@ f2fs_write_single_data_folio():
-static int f2fs_write_single_data_folio(struct folio *folio, int *submitted,
+static int f2fs_write_single_data_folio(struct folio *folio, int *submitted,
 					struct writeback_control *wbc,
 					enum iostat_type io_type,
-					u64 start, u64 end)
+					u64 *pos, u64 len)
 {
 	struct inode *inode = folio->mapping->host;
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	bool atomic_commit = f2fs_is_atomic_file(inode) &&
 					folio_test_f2fs_atomic(folio);
 	struct inode *dn_inode = atomic_commit ?
 					F2FS_I(inode)->cow_inode : inode;
-	u64 pos = folio_pos(folio);
-	pgoff_t start_idx = (start - pos) >> PAGE_SHIFT;
-	pgoff_t end_idx = (end - 1 - pos) >> PAGE_SHIFT;
+	pgoff_t start_idx = (*pos - folio_pos(folio)) >> PAGE_SHIFT;
+	pgoff_t end_idx = (*pos + len - 1 - folio_pos(folio)) >> PAGE_SHIFT;
 	int local_submitted = 0;
 	int err = 0;
@@ IPU/OPU common write_done:
 		err = f2fs_inplace_write_data(&fio);
 		if (err) {
 			if (fscrypt_inode_uses_fs_layer_crypto(inode))
 				fscrypt_finalize_bounce_page(
 						&fio.encrypted_page);
 			goto rollback_no_dnode;
 		}

 		local_submitted++;
 		set_inode_flag(inode, FI_UPDATE_WRITE);
-		continue;
+		goto write_done;
 	}
 ...
 		f2fs_outplace_write_data(&dn, &fio);
 		local_submitted++;
 		set_inode_flag(inode, FI_APPEND_WRITE);
 		trace_f2fs_do_write_data_page(folio, OPU);
 		f2fs_put_dnode(&dn);
-		continue;
+		goto write_done;
+
+write_done:
+		*pos = (u64)(data_idx + 1) << PAGE_SHIFT;
+		continue;
@@ f2fs_write_cache_folios loop + out:
 		while ((r_len = ffs_find_dirty_range(folio, &pos, end_pos))) {
-			err = f2fs_write_single_data_folio(folio, &submitted,
-					wbc, io_type, pos, pos + r_len);
+			err = f2fs_write_single_data_folio(folio, &submitted,
+					wbc, io_type, &pos, r_len);
 			folio_submitted += submitted;
 			if (err)
 				goto out;

 			nwritten += submitted;
-			pos += r_len;
 		}
 ...
 out:
-		f2fs_ffs_clear_subrange_dirty(folio, 0, folio_size(folio));
-		inode_dec_dirty_pages(inode);
+		if (f2fs_ffs_clear_subrange_dirty(folio, 0,
+					(err ? pos : end_pos) - folio_pos))
+			folio_redirty_for_writepage(wbc, folio);
+		else
+			inode_dec_dirty_pages(inode);

Btw, I think the name f2fs_write_single_data_folio() is misleading now
that it writes a dirty subrange of a large folio; would
f2fs_write_folio_dirty_range() sound better?

Thanks,
Nanzhe