Re: [f2fs-dev] [PATCH 2/2] f2fs: quota: use memalloc_nofs_{save, restore} instead of FGP_NOFS

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

On 7/6/26 22:18, Matthew Wilcox wrote:
> On Mon, Jul 06, 2026 at 05:59:43PM +0800, Chao Yu wrote:
>> FGP_NOFS could be removed later, let's use memalloc_nofs_{save,restore}
>> instead, which is recommended to be used to avoid potential deadlock
>> when memory allocation in f2fs_quota_write() will call into filesystem
>> interface again, e.g. .writepages, evict_inode, shrinker due to
>> complicated lock race condition.
> 
> I think we need to be clear on why we need the memalloc_nofs_save()
> call here.  What problem would it cause if we did call into the
> filesystem to reclaim memory?

Hmm, after taking another look at the code, I suspect below deadlock may
occur w/o NOFS flag?

User Sync (Thread 1)
- f2fs_do_quota_sync
 - f2fs_lock_op
  : down_read(cp_rwsem) [HELD] (1st Read Lock)
 - f2fs_quota_sync_file
  - dquot_writeback_dquots
   - ...
    - f2fs_quota_write
     - f2fs_write_begin
      - f2fs_filemap_get_folio
       - __filemap_get_folio
        - ... (GFP_FS allocation triggers reclaim)
         - try_to_free_pages()
          - shrink_node()
           - shrink_lruvec()
            - shrink_page_list()
             - pageout()  <-- Reclaim decides to write back a dirty F2FS data page
              - f2fs_writepages()
               - f2fs_write_cache_pages()
                - f2fs_write_single_data_page(allow_balance = true)
                 - f2fs_balance_fs(need = true)
                  - f2fs_gc()
                   - f2fs_write_checkpoint()
                    - block_operations()
                     - f2fs_lock_all()
                      : down_write(cp_rwsem) [BLOCKED] (Self-deadlock)

To Jaegeuk, please help to double check this. :)

> 
> I suspect this is the wrong place to insert this call and it should be
> near the lock that causes the problem.  I've attempted a rewrite of the
> memalloc_nofs_save documentation; let me know what you think:
> 
> /**
>  * memalloc_nofs_save - Prevent recursion into the filesystem.
>  *
>  * All memory allocations between calling this function and calling
>  * memalloc_nofs_restore() will be prevented from calling into filesystems
>  * to reclaim memory.  Clean page cache memory can still be reclaimed,
>  * but (for example) inodes will not be.
>  *
>  * The primary reason to do this is that the caller has taken a lock
>  * which would be needed by FS reclaim.  While we could theoretically
>  * call into a different filesystem in this case, it can be a deep call
>  * stack so it is better to avoid all filesystems.
>  *
>  * Filesystems often choose to incorporate a call to this function as part
>  * of starting a journal transaction.  While not a lock in the normal
>  * sense, it has much the same effect as nested journal transactions
>  * are either prohibited or expensive.
>  *
>  * Also call this function if you need to allocate memory while holding
>  * a file folio locked.  High order allocations (such as those requested
>  * by slab) can trigger compaction which will attempt to lock the folio.
>  *
>  * Context: This function is safe to be used from any context.
>  * Return: The saved flags to be passed to memalloc_nofs_restore.
>  */

It makes sense to me, thanks for updating.

Thanks,


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