From: Jan Prusakowski <jprusakowski@google.com>
To: Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org,
Jan Prusakowski <jprusakowski@google.com>,
Eric Biggers <ebiggers@google.com>,
Yonggil Song <yonggil.song@samsung.com>,
Hans Holmberg <hans.holmberg@wdc.com>
Subject: [PATCH RESEND 8/9] f2fs: compress: fix cleanup when a cluster write fails to be submitted
Date: Thu, 12 Nov 2026 14:33:54 +0000 [thread overview]
Message-ID: <20261112143355.2923939-9-jprusakowski@google.com> (raw)
In-Reply-To: <20261112143355.2923939-1-jprusakowski@google.com>
When f2fs_write_compressed_pages() fails to submit a compressed page in
the middle of a cluster, it gives up and returns -EAGAIN, and
f2fs_write_multi_pages() writes the cluster with raw pages instead. But
it leaves two things behind:
- cc->cpages[i - 1] is set to NULL before the submission is checked, so
the compressed page that was not submitted is not freed by the out_free
loop, and leaks from the compress page pool.
- The dnode already points to the new blocks of the slots before i, and
their old blocks were invalidated, while the slots from i on still
point to the old blocks. The cluster then mixes new and old compressed
data, and f2fs_write_raw_pages() sees it in that state.
Clear cc->cpages[i - 1] only after the page was submitted, and restore
the old block addresses of the slots before i, validating the old blocks
again. That's safe because they were invalidated in the same
f2fs_lock_op() section, unless they are not valid in the last checkpoint
(checkpoint=disable), where SSR may already have reused them; leave the
cluster as it is then. The old block addresses are kept on the stack for
clusters of up to 16 blocks, and in a __GFP_NOFAIL allocation of at most
1 KiB otherwise.
Also, when a raw page overwrites a compressed cluster, decrement
i_compr_blocks in f2fs_do_write_data_page() only if the page was
submitted, since the dnode keeps COMPRESS_ADDR otherwise.
Submission only fails when block allocation fails, which happens only
after the checkpoint was stopped, so all of this only keeps the
in-memory state consistent until unmount. Note that if the allocation
fails while opening a new segment, f2fs_allocate_data_block() has
already updated the SIT for the block it doesn't return, and that is
not undone here.
Fixes: 4c8ff7095bef ("f2fs: support data compression")
Assisted-by: LLM
Signed-off-by: Jan Prusakowski <jprusakowski@google.com>
---
fs/f2fs/compress.c | 61 +++++++++++++++++++++++++++++++++++++++++++---
fs/f2fs/data.c | 6 ++---
2 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index c5758da7c621..0147a7e05022 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1279,6 +1279,45 @@ int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock)
return f2fs_do_truncate_blocks(inode, round_up(from, PAGE_SIZE), lock);
}
+/*
+ * Undo the block address updates of the first @nr slots of a cluster, after a
+ * write of the cluster failed to be submitted in the middle. Submission only
+ * fails if block allocation fails, i.e. after the checkpoint was stopped, so
+ * this only keeps the in-memory state consistent until unmount.
+ *
+ * The old blocks were invalidated in the same f2fs_lock_op() section, so they
+ * can be validated again, unless SSR could have reused them because they are
+ * not valid in the last checkpoint (checkpoint=disable). Leave the cluster as
+ * it is then.
+ */
+static void f2fs_undo_cluster_update(struct dnode_of_data *dn,
+ block_t *old_blkaddr, int nr)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
+ int i;
+
+ for (i = 0; i < nr; i++) {
+ if (!f2fs_is_checkpointed_data(sbi, old_blkaddr[i]))
+ return;
+ }
+
+ for (i = 0; i < nr; i++, dn->ofs_in_node++) {
+ block_t old_blk = old_blkaddr[i];
+ block_t new_blk = f2fs_data_blkaddr(dn);
+
+ if (new_blk == old_blk)
+ continue;
+ f2fs_update_data_blkaddr(dn, old_blk);
+ if (__is_valid_data_blkaddr(new_blk))
+ f2fs_invalidate_blocks(sbi, new_blk, 1);
+ if (__is_valid_data_blkaddr(old_blk))
+ f2fs_revalidate_blocks(sbi, old_blk, 1);
+ }
+}
+
+/* Old block addresses of clusters up to this size are kept on the stack. */
+#define F2FS_ONSTACK_CLUSTER_SIZE 16
+
static int f2fs_write_compressed_pages(struct compress_ctx *cc,
int *submitted,
struct writeback_control *wbc,
@@ -1306,6 +1345,8 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
struct node_info ni;
struct compress_io_ctx *cic;
struct f2fs_lock_context lc;
+ block_t old_blkaddr_onstack[F2FS_ONSTACK_CLUSTER_SIZE];
+ block_t *old_blkaddr = old_blkaddr_onstack;
pgoff_t start_idx = start_idx_of_cluster(cc);
unsigned int last_index = cc->cluster_size - 1;
loff_t psize;
@@ -1363,6 +1404,12 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
cic->nr_rpages = cc->cluster_size;
+ if (cc->cluster_size > ARRAY_SIZE(old_blkaddr_onstack)) {
+ /* At most 1 KiB, don't fail the write for it. */
+ old_blkaddr = kmalloc_array(cc->cluster_size, sizeof(block_t),
+ GFP_NOFS | __GFP_NOFAIL);
+ }
+
for (i = 0; i < cc->valid_nr_cpages; i++) {
f2fs_set_compressed_page(cc->cpages[i], inode,
page_folio(cc->rpages[i + 1])->index, cic);
@@ -1386,6 +1433,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
blkaddr = f2fs_data_blkaddr(&dn);
fio.folio = page_folio(cc->rpages[i]);
fio.old_blkaddr = blkaddr;
+ old_blkaddr[i] = blkaddr;
/* cluster header */
if (i == 0) {
@@ -1412,14 +1460,16 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
fio.compressed_page = cc->cpages[i - 1];
- cc->cpages[i - 1] = NULL;
fio.submitted = 0;
f2fs_outplace_write_data(&dn, &fio);
if (unlikely(!fio.submitted)) {
cancel_cluster_writeback(cc, cic, i);
*submitted = 0;
- goto out_free_page_array;
+ dn.ofs_in_node -= i;
+ f2fs_undo_cluster_update(&dn, old_blkaddr, i);
+ goto out_submit_err;
}
+ cc->cpages[i - 1] = NULL;
(*submitted)++;
unlock_continue:
inode_dec_dirty_block(cc->inode);
@@ -1431,6 +1481,9 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
f2fs_i_compr_blocks_update(inode, cc->valid_nr_cpages, true);
add_compr_block_stat(inode, cc->valid_nr_cpages);
+ if (old_blkaddr != old_blkaddr_onstack)
+ kfree(old_blkaddr);
+
set_inode_flag(cc->inode, FI_APPEND_WRITE);
f2fs_put_dnode(&dn);
@@ -1450,7 +1503,9 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
f2fs_destroy_compress_ctx(cc, false);
return 0;
-out_free_page_array:
+out_submit_err:
+ if (old_blkaddr != old_blkaddr_onstack)
+ kfree(old_blkaddr);
page_array_free(sbi, cic->rpages, cc->cluster_size);
out_put_cic:
kmem_cache_free(cic_entry_slab, cic);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 4d7731fbf5a4..016aa347880e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -4018,11 +4018,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
pending_added = true;
}
- if (fio->compr_blocks && fio->old_blkaddr == COMPRESS_ADDR)
- f2fs_i_compr_blocks_update(inode, fio->compr_blocks - 1, false);
-
/* LFS mode write path */
f2fs_outplace_write_data(&dn, fio);
+ if (fio->compr_blocks && fio->old_blkaddr == COMPRESS_ADDR &&
+ likely(fio->submitted))
+ f2fs_i_compr_blocks_update(inode, fio->compr_blocks - 1, false);
if (unlikely(opu_entry)) {
opu_entry->new_blkaddr[0] = fio->new_blkaddr;
f2fs_end_dio_opu_track(inode, opu_entry, fio->submitted, false);
--
2.56.0.567.gdb80ae8cd5-goog
next prev parent reply other threads:[~2026-11-12 14:38 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-12 14:33 [PATCH RESEND 0/9] f2fs: fix stale data exposure on failed O_DIRECT writes Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 1/9] f2fs: compress: avoid sleeping in atomic context in f2fs_decompress_cluster() Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 2/9] f2fs: compress: abort on read error in prepare_compress_overwrite() Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 3/9] f2fs: keep old block address when out-of-place write fails to allocate Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 4/9] f2fs: propagate errors from buffered fallback of O_DIRECT writes Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 5/9] f2fs: truncate blocks allocated past EOF by a failed direct write Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 6/9] f2fs: roll back failed block updates of O_DIRECT buffered fallback Jan Prusakowski
2026-11-12 14:33 ` [PATCH RESEND 7/9] f2fs: roll back failed block updates of direct writes Jan Prusakowski
2026-11-12 14:33 ` Jan Prusakowski [this message]
2026-11-12 14:33 ` [PATCH RESEND 9/9] f2fs: compress: roll back failed cluster writes of O_DIRECT buffered fallback Jan Prusakowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261112143355.2923939-9-jprusakowski@google.com \
--to=jprusakowski@google.com \
--cc=chao@kernel.org \
--cc=ebiggers@google.com \
--cc=hans.holmberg@wdc.com \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=yonggil.song@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®