Re: [PATCH v4 07/16] f2fs: convert the ->private flag helpers to folio-only

[Zi Yan <ziy@nvidia.com>]

On 13 Sep 2026, at 22:24, Zi Yan wrote:

> page-based ->private flag helpers are used in the compression path, where
> large folios are not enabled. They can use folio versions with
> page_folio(). The two remaining users in data.c and segment.c can use
> fio->folio instead of fio->page (two are in a union).
>
> Drop page-based helpers after the conversion and rename
> PAGE_PRIVATE_{GET,SET,CLEAR}_FUNC() and the PAGE_PRIVATE_* flags to
> F2FS_FOLIO_PRIVATE_* to match. Convert the folio/page union from
> f2fs_io_info union to folio only, since no page user is left.
>
> The folio helpers do a plain read-modify-write where the page ones used
> set_bit()/clear_bit(). It is fine because the converted code either holds
> folio lock or, in f2fs_compress_write_end_io(), matches what the
> non-compressed code does in f2fs_write_end_bio().
>
> Assisted-by: LLM
> Suggested-by: Tal Zussman <tz2294@columbia.edu>
> Co-developed-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> To: Jaegeuk Kim <jaegeuk@kernel.org>
> To: Chao Yu <chao@kernel.org>
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Cc: linux-kernel@vger.kernel.org
> Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>  fs/f2fs/compress.c | 35 ++++++++++++-------
>  fs/f2fs/data.c     |  2 +-
>  fs/f2fs/f2fs.h     | 99 +++++++++++++++++++++---------------------------------
>  fs/f2fs/segment.c  |  2 +-
>  4 files changed, 63 insertions(+), 75 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index ce88092d9ce26..09d9b8d0fdcce 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1064,13 +1064,15 @@ static void cancel_cluster_writeback(struct compress_ctx *cc,
>
>  	/* Cancel writeback and stay locked. */
>  	for (i = 0; i < cc->cluster_size; i++) {
> +		struct folio *folio = page_folio(cc->rpages[i]);
> +
>  		if (i < submitted) {
>  			inode_inc_dirty_pages(cc->inode);
> -			lock_page(cc->rpages[i]);
> +			folio_lock(folio);
>  		}
> -		clear_page_private_gcing(cc->rpages[i]);
> -		if (folio_test_writeback(page_folio(cc->rpages[i])))
> -			end_page_writeback(cc->rpages[i]);
> +		folio_clear_f2fs_gcing(folio);
> +		if (folio_test_writeback(folio))
> +			folio_end_writeback(folio);
>  	}
>  }

Sashiko[1] said there is a pre-existing deadlock here. Submitted
pages are unlocked and remain under writeback. If someone else locks
one of such pages and waits for writeback to be completed, like
truncate_inode_pages_range(), they can wait forever, since
cancel_cluster_writeback() can end the writeback only after it locks
the page.

Answer:

Yes. I have submitted a patch[2] for comments.

[1] https://sashiko.dev/#/patchset/73521?part=7
[2] https://lore.kernel.org/all/862E7DC4-14D6-48AE-87C8-11664354718C@nvidia.com/


<snip>

>
> -#define PAGE_PRIVATE_SET_FUNC(name, flagname) \
> +#define F2FS_FOLIO_PRIVATE_SET_FUNC(name, flagname) \
>  static inline void folio_set_f2fs_##name(struct folio *folio)		\
>  {									\
> -	unsigned long v = (1UL << PAGE_PRIVATE_NOT_POINTER) |		\
> -			     (1UL << PAGE_PRIVATE_##flagname);		\
> +	unsigned long v = (1UL << F2FS_FOLIO_PRIVATE_NOT_POINTER) |	\
> +			     (1UL << F2FS_FOLIO_PRIVATE_##flagname);	\
>  	if (!folio->private)						\
>  		folio_attach_private(folio, (void *)v);			\
>  	else {								\
>  		v |= (unsigned long)folio->private;			\
>  		folio->private = (void *)v;				\
>  	}								\
> -}									\

Sashiko also complained that folio version is a non-atomic read-modify-write
sequence, whereas the page version uses atomic bit ops. During IO completion,
f2fs_write_end_bio() executes in softirq context and calls
folio_clear_f2fs_gcing() without holding the folio lock. Concurrently,
a user process dirtying an mmapped F2FS atomic file could trigger
folio_set_f2fs_atomic() in process context. Even though the process holds
the folio lock, the softirq can interrupt the CPU during this non-atomic
assignment sequence. This is racy and can cause a silent drop of ATOMIC_WRITE
flag.

Answer:

The above commit message talked about it. It matches the non-compressed code
path. In addition, atomic-write files cannot be compressed, so
folio_set_f2fs_atomic() never runs on a compressed folio.

Best Regards,
Yan, Zi