Re: [f2fs-dev] [PATCH] f2fs: Use mapping->gfp_mask to get file cache for writing

[Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 9/30/2025 11:17 AM, Jiucheng Xu wrote:
> 
> 
> On 9/24/2025 10:25 AM, Chao Yu wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On 9/18/25 11:32, Jiucheng Xu via B4 Relay wrote:
>>> From: Jiucheng Xu <jiucheng.xu@amlogic.com>
>>>
>>> On 32-bit architectures, when GFP_NOFS is used, the file cache for write
>>> operations cannot be allocated from the highmem and CMA.
>>
>> Hi,
>>
>> Have you suffered any problem w/o this patch? Can you please describe more
>> details about it?
>>
> Hi Chao,
> 
> Thanks for your comments.
> 
> Our a platform uses a 1.5G DRAM, and the kernel is aarch32.
> We have a critical scenario where system need to record multimedia data
> while replaying the previously recorded multimedia files. However,
> stuttering often occurs during playback. The problem does not arise on
> aarch64 platforms with the same memory size.
> 
> We have analyzed the root cause as follows:
> Data written using GFP_NOFS is only allocated from normal memory. Since
> the normal memory is only 768MB, it easily triggers the kswapd to
> reclaim memory, which in turn reclaims and clears the file cache of the
> recorded data. As a result, during playback, the system fails to hit the
> file cache and thus has to re-read data directly from the storage. Given
> that our storage has relatively poor performance, concurrent read
> (playback) and write (recording) operations lead to significant IO
> latency. High read latency then causes stuttering during playback.

Hi Jiucheng,

Thanks for the explanation.

>>>
>>> Since mapping->gfp_mask is set to GFP_HIGHUSER_MOVABLE during inode
>>
>> GFP_HIGHUSER_MOVABLE includes __GFP_FS, we should avoid using __GFP_FS here.
>> f2fs_write_begin() uses GFP_NOFS like the most of other filesystem to avoid
>> potential deadlock, as __filemap_get_folio(, .. |__GFP_FS | ..) may run into
>> memory reclaim to call ->writeback in where we may suffer deadlock potentially.
>>
> Since our platform only support 5.15 kernel, I have checked
> EXT4/FAT/ntfs3 and find they all use mapping_gfp_mask(mapping)) as GFP

Yes, but also we can see iomap gets rid of using __GFP_FS by default:

struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos, size_t len)
{
	fgf_t fgp = FGP_WRITEBEGIN | FGP_NOFS;

	if (iter->flags & IOMAP_NOWAIT)
		fgp |= FGP_NOWAIT;
	if (iter->flags & IOMAP_DONTCACHE)
		fgp |= FGP_DONTCACHE;
	fgp |= fgf_set_order(len);

	return __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT,
			fgp, mapping_gfp_mask(iter->inode->i_mapping));
}

> flag to get page cache on kernel 5.15:
> 
> 6100cca:
> ___GFP_HIGHMEM |___GFP_MOVABLE | ___GFP_IO | ___GFP_FS
> |___GFP_DIRECT_RECLAIM |___GFP_KSWAPD_RECLAIM |___GFP_HARDWALL
> |___GFP_SKIP_KASAN_UNPOISON | ___GFP_SKIP_KASAN_POISON
> 
> therefor that's why I recommend this flag.
> 
> I'm not sure if the above flags has some problems on f2fs, so I submit
> it and would be very eager to learn about the views of your experts.

IIRC, f2fs uses GFP_NOFS in ->write_begin for long time w/o any problem, in order
to avoid any potential issue by using ___GFP_FS, and also to consider allowing
memory allocation w/ ___GFP_HIGHMEM & ___GFP_MOVABLE, what about changing as below:

folio = __filemap_get_folio(mapping, index,
			FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_NOFS,
			mapping_gfp_mask(mapping));

Thanks,

> 
> Thanks and Best Regards,
> Jiucheng
> 
>> Thanks,
>>
>>> allocation, using mapping_gfp_mask(mapping) as the GFP flag of getting file
>>> cache for writing is more efficient for 32-bit architectures.
>>>> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
>>> ---
>>>    fs/f2fs/data.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 7961e0ddfca3aaa332b7dbd4985ae7766551834f..9fbc41f9accb2626da22754f1a424da4805ca823 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -3587,7 +3587,8 @@ static int f2fs_write_begin(const struct kiocb *iocb,
>>>          * Will wait that below with our IO control.
>>>          */
>>>         folio = __filemap_get_folio(mapping, index,
>>> -                             FGP_LOCK | FGP_WRITE | FGP_CREAT, GFP_NOFS);
>>> +                             FGP_LOCK | FGP_WRITE | FGP_CREAT,
>>> +                             mapping_gfp_mask(mapping));
>>>         if (IS_ERR(folio)) {
>>>                 err = PTR_ERR(folio);
>>>                 goto fail;
>>>
>>> ---
>>> base-commit: c872b6279cd26762339ff02513e2a3f16149a6f1
>>> change-id: 20250910-origin-dev-8a5ff6bee1f2
>>>
>>> Best regards,
>>
> 



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel