Re: [PATCH] f2fs: fix to prevent clearing immutable for large folio supported inode

[Chao Yu <chao@kernel.org>]

On 1/9/2026 11:05 AM, Barry Song wrote:
> On Fri, Jan 9, 2026 at 3:47 PM Chao Yu <chao@kernel.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@kernel.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

    # 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?

> 
> 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.

Thanks,

> 
> 
>> +       }
>> +
>>          fi->i_flags = iflags | (fi->i_flags & ~mask);
>>          f2fs_bug_on(F2FS_I_SB(inode), (fi->i_flags & F2FS_COMPR_FL) &&
>>                                          (fi->i_flags & F2FS_NOCOMP_FL));
>> --
>> 2.40.1
> 
> 
> Thanks
> Barry