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

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")

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

								Honza
-- 
Jan Kara <[email protected]>
SUSE Labs, CR


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