Re: [f2fs-dev] [DISCUSSION] f2fs: compatibility between compressed files and large folios

Nanzhe,

Thanks for the proposal.

On 7/29/26 08:28, Nanzhe Zhao wrote:
> Hi all,
> 
> Chao Yu and I discussed the compatibility between compressed files and large
> folios.
> 
> The main race scenario is that f2fs_new_inode() creates an inode without the
> compression flag and enables large-folio support for its mapping. Later,
> f2fs_setflags_common() can set the compression flag while large-folio support
> is already enabled for the inode. Neither f2fs_iget() nor f2fs_new_inode()
> can prevent this for an active inode.
> 
> Here are the two approaches I am considering.
> 
> 1. Reject setting the compression flag on a large-folio mapping
> 
> We can reject setting the compression flag in f2fs_setflags_common() if the
> inode mapping supports large folios:
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2250,6 +2250,9 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>          if ((iflags ^ masked_flags) & F2FS_COMPR_FL) {
>                  if (masked_flags & F2FS_COMPR_FL) {
>                          if (!f2fs_disable_compressed_file(inode))
>                                  return -EINVAL;
>                  } else {
> +                       if (mapping_large_folio_support(inode->i_mapping))
> +                               return -EOPNOTSUPP;
> +
>                          /* try to convert inline_data to support compression */
>                          int err = f2fs_convert_inline_inode(inode);
>                          if (err)
>                                  return err;
> 
> Another implementation is:
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2250,6 +2250,9 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>          if ((iflags ^ masked_flags) & F2FS_COMPR_FL) {
>                  if (masked_flags & F2FS_COMPR_FL) {
>                          if (!f2fs_disable_compressed_file(inode))
>                                  return -EINVAL;
>                  } else {
> +                       if (IS_ENABLED(CONFIG_F2FS_LARGE_FOLIO))
> +                               return -EOPNOTSUPP;
> +
>                          /* try to convert inline_data to support compression */
>                          int err = f2fs_convert_inline_inode(inode);
>                          if (err)
>                                  return err;
> 
> The concern with this approach is that new files trying to set compression
> through the ioctl will be rejected, so compression will not be available. I
> think we can document that enabling large-folio support in f2fs disables
> compression.
> 
> 2. Keep the compression flag and restore order 0
> 
> If the compression flag and compression functionality need to remain
> available, we can restore the mapping order to 0 after set_compress_context()
> successfully sets the compression flag in f2fs_setflags_common():
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2268,6 +2268,10 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
>                                  return -EINVAL;
>                          }
>                          err = set_compress_context(inode);
> +                       if (!err &&
> +                           mapping_large_folio_support(inode->i_mapping))
> +                               mapping_set_folio_order_range(
> +                                       inode->i_mapping, 0, 0);

I prefer this solution, it keeps f2fs compression functionality available, rather
than just disabling it after we enable large folio. I suspect there is existing
users: applications are using it, I don't want to break any userspace use.

>                          f2fs_up_write(&fi->i_sem);
> 
>                          if (err)
>                                  return err;
> 
> Under normal Android workloads, a file whose compression flag can be set by
> f2fs_setflags_common() has no data blocks. Also, an application will not read
> a newly created file with size 0 and fill the page cache with zeroed large
> folios. Therefore, setting the mapping order to 0 here is safe.
> 
> A theoretical sequence is: create a file, use ftruncate() to extend it to a
> non-zero size without allocating data blocks, read the file holes so that
> zeroed large folios are left in the page cache, and then set the compression
> flag through the ioctl. If truncate_inode_pages() is not called before the
> mapping order is reset, the existing large folios remain in the page cache.
> Also, since mapping_set_folio_order_range() is not atomic, readahead may
> theoretically access the mapping's folio-order flags without holding the inode
> lock. Neither case is expected under normal Android workloads.

Can we call truncate_inode_pages() after mapping_set_folio_order_range()? something
like this:

		filemap_invalidate_lock

		mapping_set_folio_order_range(, 0, 0)
		truncate_inode_pages()

		if (mapping->nrpages) {
			err = -EBUSY;
			goto out_unlock;
		}

		...
		err = set_compress_context(inode);
		...

		filemap_invalidate_unlock

Thanks,

> 
> Jaegeuk previously mentioned that a file could have both the compression flag
> and a large-folio mapping. Its writeback could use the normal large-folio path
> without actually compressing the file. After the inode is evicted and the file
> is opened again, f2fs_iget() would see the compression flag, prevent the
> large-folio mapping from being enabled, and return to the normal order-0 read
> path. Chao Yu pointed out that it would be functionally strange if a file
> with the compression flag was not compressed during its first writeback.
> 
> Please let us know which of the above approaches you think is better, or
> whether we have another better approach.
> 
> Thanks,
> 
> Nanzhe
> 



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel