Message-ID: <CAGsJ_4weE5T1uBA1-JoJdbZY4E91eN_OL3hMdqixLcoKSrmuzw@mail.gmail.com>
Date: Fri, 9 Jan 2026 16:57:58 +1300
From: Barry Song <21cnbao@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, linux-f2fs-devel@...ts.sourceforge.net, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] f2fs: fix to prevent clearing immutable for large folio
 supported inode

On Fri, Jan 9, 2026 at 4:45 PM Chao Yu <chao@...nel.org> wrote:
>
> On 1/9/2026 11:05 AM, Barry Song wrote:
> > On Fri, Jan 9, 2026 at 3:47 PM Chao Yu <chao@...nel.org> wrote:
> >>
> >> Below testcase can change large folio supported inode from immutable
> >> to mutable, it's not as expected, let's add a new check condition in
> >> f2fs_setflags_common() to detect and reject it.
> >>
> >> - dd if=/dev/zero of=/mnt/f2fs/test bs=32k count=4
> >> - f2fs_io setflags immutable /mnt/f2fs/test
> >> - echo 3 > /proc/sys/vm/drop_caches
> >>   : to reload inode with large folio
> >> - f2fs_io read 32 0 1 mmap 0 0 /mnt/f2fs/test
> >> - f2fs_io clearflags immutable /mnt/f2fs/test
> >>
> >> Signed-off-by: Chao Yu <chao@...nel.org>
> >> ---
> >>   fs/f2fs/file.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >> index ce291f152bc3..f7f9da0b215f 100644
> >> --- a/fs/f2fs/file.c
> >> +++ b/fs/f2fs/file.c
> >> @@ -2155,6 +2155,12 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
> >>                  }
> >>          }
> >>
> >> +       if ((iflags ^ masked_flags) & F2FS_IMMUTABLE_FL) {
> >> +               if ((masked_flags & F2FS_IMMUTABLE_FL) &&
> >> +                       mapping_large_folio_support(inode->i_mapping))
> >> +                       return -EINVAL;
> >
>
> Barry,
>
> I noticed that we are allowed to clear immutable if large folio are supported
> in target inode, since we can prevent to open w/ write permission in ->open after
> immutable is cleared, see details in f2fs doc below.
>
> So, anyway, I guess my patch should be ignored.
>
> Per-file Read-Only Large Folio Support
> --------------------------------------
>
> F2FS implements large folio support on the read path to leverage high-order
> page allocation for significant performance gains. To minimize code complexity,
> this support is currently excluded from the write path, which requires handling
> complex optimizations such as compression and block allocation modes.
>
> This optional feature is triggered only when a file's immutable bit is set.
> Consequently, F2FS will return EOPNOTSUPP if a user attempts to open a cached
> file with write permissions, even immediately after clearing the bit. Write
> access is only restored once the cached inode is dropped. The usage flow is
> demonstrated below:
>
> .. code-block::
>
>     # f2fs_io setflags immutable /data/testfile_read_seq
>
>     /* flush and reload the inode to enable the large folio */
>     # sync && echo 3 > /proc/sys/vm/drop_caches
>
>     /* mmap(MAP_POPULATE) + mlock() */
>     # f2fs_io read 128 0 1024 mmap 1 0 /data/testfile_read_seq
>
>     /* mmap() + fadvise(POSIX_FADV_WILLNEED) + mlock() */
>     # f2fs_io read 128 0 1024 fadvise 1 0 /data/testfile_read_seq
>
>     /* mmap() + mlock2(MLOCK_ONFAULT) + madvise(MADV_POPULATE_READ) */
>     # f2fs_io read 128 0 1024 madvise 1 0 /data/testfile_read_seq
>
>     # f2fs_io clearflags immutable /data/testfile_read_seq
>
>     # f2fs_io write 1 0 1 zero buffered /data/testfile_read_seq
>     Failed to open /mnt/test/test: Operation not supported
>
>     /* flush and reload the inode to disable the large folio */
>     # sync && echo 3 > /proc/sys/vm/drop_caches

Right, I see. The only concern is that this would require dropping
the page cache for the entire system, rather than for a single inode.


>
>     # f2fs_io write 1 0 1 zero buffered /data/testfile_read_seq
>     Written 4096 bytes with pattern = zero, total_time = 29 us, max_latency = 28 us
>
>     # rm /data/testfile_read_seq
>
> > Hi Yu, I find it a bit odd to prevent unsetting immutable solely
> > because large folios are in use. If unsetting immutable is considered
> > unexpected behavior, it should be disallowed regardless of whether
> > large folios are used, and apply equally in both cases.
>
> To confirm, you mean if clearing immutable is considered unexpected behavior,
> we need to prevent clearing immutable for inode which doesn't enable large folio?

Right. It feels unfair to prevent clearing immutable solely because
the file happens to have large folios.

>
> >
> > I'm not sure whether reverting the large folios setting is the
> > better approach:
> > truncate_pagecache(inode, inode->i_size);
> > mapping_set_folio_order_range(inode->i_mapping, 0, 0);
>
> If we want to support reverting the large folios setting dynamically as you
> proposed above, it need to consider more race case and corner case, so, a
> little bit complicated.

Right. The idea is to truncate the page cache via
truncate_pagecache(inode, 0) and set the maximum page-cache order
to 0. That said, we still need to consider whether any related
locks are held.

>From the comment, it seems that we may need to hold i_rwsem and
invalidate_lock.

/**
 * truncate_inode_pages - truncate *all* the pages from an offset
 * @mapping: mapping to truncate
 * @lstart: offset from which to truncate
 *
 * Called under (and serialised by) inode->i_rwsem and
 * mapping->invalidate_lock.
 *
 * ...
 */
void truncate_inode_pages(struct address_space *mapping, loff_t lstart)
{
        truncate_inode_pages_range(mapping, lstart, (loff_t)-1);
}
EXPORT_SYMBOL(truncate_inode_pages);


If clearing immutable is indeed rare, we may leave this as is, since
writes are not supported until the page cache is fully dropped.
Eventually, we will support large folios on non-immutable files.

Thanks
Barry

Powered by blists - more mailing lists