[PATCH 1/2] f2fs: quota: do not use GFP_NOFS in f2fs_quota_read()

[PATCH 1/2] f2fs: quota: do not use GFP_NOFS in f2fs_quota_read()

[Chao Yu]

Commit 02117b8ae9c0 ("f2fs: Set GF_NOFS in read_cache_page_gfp while doing
f2fs_quota_read") adds GFP_NOFS in f2fs_quota_read() to avoid below deadlock:

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

As Jan Kara mentioned, quota system has fixed this issue w/ commit
537e11cdc7a6 ("quota: Prevent memory allocation recursion while holding
dq_lock"), so this GFP_NOFS flag should be relic, let's drop it.

Cc: Jan Kara <jack@suse.cz>
Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index da468df058eb..fdfd6a7203dd 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3167,8 +3167,7 @@ static ssize_t f2fs_quota_read(struct super_block *sb, int type, char *data,
 		size_t offset;
 
 repeat:
-		folio = mapping_read_folio_gfp(mapping, off >> PAGE_SHIFT,
-				GFP_NOFS);
+		folio = mapping_read_folio_gfp(mapping, off >> PAGE_SHIFT, 0);
 		if (IS_ERR(folio)) {
 			if (PTR_ERR(folio) == -ENOMEM) {
 				memalloc_retry_wait(GFP_NOFS);
-- 
2.49.0

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

[Chao Yu]

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.

Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/data.c  | 2 +-
 fs/f2fs/super.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ac1cf4de3d62..be4c1d4ed6b2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3994,7 +3994,7 @@ static int f2fs_write_begin(const struct kiocb *iocb,
 	 * Will wait that below with our IO control.
 	 */
 	folio = f2fs_filemap_get_folio(mapping, index,
-				FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_NOFS,
+				FGP_LOCK | FGP_WRITE | FGP_CREAT,
 				mapping_gfp_mask(mapping));
 	if (IS_ERR(folio)) {
 		err = PTR_ERR(folio);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index fdfd6a7203dd..d28a93657658 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3215,13 +3215,16 @@ static ssize_t f2fs_quota_write(struct super_block *sb, int type,
 	void *fsdata = NULL;
 	int err = 0;
 	int tocopy;
+	unsigned int nofs_flags;
 
 	while (towrite > 0) {
 		tocopy = min_t(unsigned long, sb->s_blocksize - offset,
 								towrite);
 retry:
+		nofs_flags = memalloc_nofs_save();
 		err = a_ops->write_begin(NULL, mapping, off, tocopy,
 							&folio, &fsdata);
+		memalloc_nofs_restore(nofs_flags);
 		if (unlikely(err)) {
 			if (err == -ENOMEM) {
 				memalloc_retry_wait(GFP_NOFS);
-- 
2.49.0

Re: [PATCH 1/2] f2fs: quota: do not use GFP_NOFS in f2fs_quota_read()

[Matthew Wilcox]

On Mon, Jul 06, 2026 at 05:59:42PM +0800, Chao Yu wrote:
> -		folio = mapping_read_folio_gfp(mapping, off >> PAGE_SHIFT,
> -				GFP_NOFS);
> +		folio = mapping_read_folio_gfp(mapping, off >> PAGE_SHIFT, 0);

0?  Surely you mean GFP_KERNEL?

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

[Matthew Wilcox]

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?

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.
 */