From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1CB9B500966 for ; Fri, 9 Jan 2026 03:45:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767930319; cv=none; b=X2p5/jZBgQQ3tdd910x9QfWWXGCitxXMIY4dGhM51EY3T9j6jGjmNLQL78JzZMAo7PFAjwEUK1JlooNdFwFV2EbNbzTDuKD8v/TZTJ/E2JItA9qSS7tlpjbdMNFadLSkxrugMKLAL9jnz/P88oVibFQfWEt6rlAdFQMI4OZWeII= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767930319; c=relaxed/simple; bh=3swegdi4GX1qhshed/ec9WKo4JfH5/Z4kN3dIpPld4s=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=KNAxEN3eEZ0lvZsSeZqM4HvL3TdOYKCJsqhW7e75ryUPKcP4AMUGMLFLAbcp4MERx/CCoV34o4SwoctUzEIlD5mIXtCELDhQldeHgBw2qVkZQn18C8kEx+aklkxIWMax8PuS1u9UCJv63weUEXmNJ6N6+NSuJkihddCkkot9oLQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TU72MC9C; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TU72MC9C" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9B29AC4CEF1; Fri, 9 Jan 2026 03:45:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1767930318; bh=3swegdi4GX1qhshed/ec9WKo4JfH5/Z4kN3dIpPld4s=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=TU72MC9CtVbX3V38b8OESX+W8SUG46XnZqIvSTcfd7Nv5sJxzzzNwSWoaTGWNX4qe O51ssSCGh+2TMVO2fthlvX5wXTmlKdaXEBLIsbQnqKfgFXb7vF6hH5ISaDtLqBSZvQ JuVKfyr6onMtkq/USsHdtfy8sYoZ66JhUwq7PDhrNLt9heflMwSv5cThfqu6PiNGgK RoR2j/7OQ7oOlcz8NHAw1JS04R/uVzE5dDQsbT3H+h1q79gVxGrrlQ1OY1YRPb6bnv ZMz5FhK/J8tIyY4D4YBAK1K2/S45WLG30cLUnP0sKPxCRi95S9rN7S2zg9SLFRm1fN eLKkbVHvgQyyg== Message-ID: <2cdd4281-7e37-4dd0-91b1-1ab16ef772aa@kernel.org> Date: Fri, 9 Jan 2026 11:45:29 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org, jaegeuk@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH] f2fs: fix to prevent clearing immutable for large folio supported inode To: Barry Song <21cnbao@gmail.com> References: <20260109024716.12047-1-chao@kernel.org> Content-Language: en-US From: Chao Yu In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 1/9/2026 11:05 AM, Barry Song wrote: > On Fri, Jan 9, 2026 at 3:47 PM Chao Yu 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 >> --- >> 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