Re: [f2fs-dev] Use of FGP_NOFS in f2fs

On 6/29/26 16:58, Jan Kara wrote:
> On Mon 29-06-26 04:54:38, Matthew Wilcox wrote:
>> [Adding Jan Kara; retaining whole message for context]
> 
> Thanks!
> 
>> On Mon, Jun 29, 2026 at 09:39:25AM +0800, Chao Yu wrote:
>>> On 6/26/26 17:30, Jiucheng Xu wrote:
>>>> On 6/25/2026 3:17 AM, Matthew Wilcox wrote:
>>>>> [ EXTERNAL EMAIL ]
>>>>> I am trying to remove FGP_NOFS from the kernel and the last remaining
>>>>> user was added by you last year in commit 2308de27c03d.  I'm trying to
>>>>> understand why.  Did you see an actual problem if you do not use it,
>>>>> or was it theoretical?  The commit message says "to avoid potential
>>>>> deadlock issues", but it's not clear to me whether you know they are
>>>>> there, or only think they are there.
>>>>>
>>>>> I'd really like to understand what the issues are as the normal issues
>>>>> which make calling into the filesystem (holding another folio locked,
>>>>> holding a mutex relied upon by reclaim) don't seem to be present.
>>>>> So if there is something, I'm not sure what it is.
>>>> Hi Matthew,
>>>>
>>>> The FGP_NOFS flag was suggested by Chao and not adding it might lead to deadlock issues.
>>>>
>>>> @Chao could you explain this for Matthew?
>>>
>>> Hi Matthew, Jiucheng,
>>>
>>> IIRC, for normal path from write -> write_begin, it seems fine since there is
>>> no f2fs-specified mutex or other folio lock in the path, but I was worried about
>>> quota path when I suggested to keep GFP_NOFS flag, as the lock race condition
>>> is quite complicated there: f2fs internal lock vs quota system lock vs folio lock...
>>>
>>> I searched the commits and found this:
>>>
>>> Commit 02117b8ae9c0 ("f2fs: Set GF_NOFS in read_cache_page_gfp while doing
>>> f2fs_quota_read")
>>>
>>> In this case, f2fs_quota_read() was calling read_mapping_page(), which allowed
>>> GFP_FS allocations. This triggered the following deadlock:
>>>
>>>     Thread 1 (User Open/Write)
>>>     - do_sys_open
>>>      - vfs_open
>>>       - dquot_file_open
>>>        - dquot_initialize
>>>         - dqget
>>>          - dquot_acquire
>>>           : locks &dqopt->dqio_mutex (VFS Quota Mutex)
>>>           - qtree_read_dquot
>>>            - f2fs_quota_read
>>>             - read_mapping_page (GFP_KERNEL / allows GFP_FS)
>>>              - __alloc_pages_nodemask
>>>               - try_to_free_pages (Direct Reclaim)
>>>                - prune_icache_sb
>>>                 - evict
>>>                  - f2fs_evict_inode
>>>                   - dquot_drop
>>>                    - dqput
>>>                     - dquot_commit
>>>                      : tries to lock &dqopt->dqio_mutex again
>>>                      ==> DEADLOCK (waiting for itself)
>>>
>>> It may trigger deadlock in f2fs_quota_write() as the same way in f2fs_quota_read()
>>> path? let me know if I'm missing anything.
>>>
>>> Are memalloc_nofs_save() and memalloc_nofs_restore() recommended for such case?
>>> any suggestions?
>>
>> In general, yes, memalloc_nofs_save() after locking dqio_mutex sounds like
>> the right idea to me.  I'd want to familiarise myself more thoroughly
>> with the code before making a firm recommendation, and it's probably
>> quicker to just ask Jan ;-)
> 
> So the above stacktrace shouldn't be possible since you've added
> memalloc_nofs_save() / restore() pairs into quota code in:
> 
> 537e11cdc7a6 ("quota: Prevent memory allocation recursion while holding dq_lock")

Ah, I see, it has been fixed in quota system.

> 
> Chao, are you sure the GFP_NOFS mask isn't just a relic from the past?

So, I agree it's a relic implementation, thanks Jan for reminder.

Thanks,

> 
> 								Honza



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