Skip to content

Commit 0c6ca06

Browse files
Dave ChinnerChandan Babu R
authored andcommitted
xfs: quota radix tree allocations need to be NOFS on insert
In converting the XFS code from GFP_NOFS to scoped contexts, we converted the quota radix tree to GFP_KERNEL. Unfortunately, it was not clearly documented that this set was because there is a dependency on the quotainfo->qi_tree_lock being taken in memory reclaim to remove dquots from the radix tree. In hindsight this is obvious, but the radix tree allocations on insert are not immediately obvious, and we avoid this for the inode cache radix trees by using preloading and hence completely avoiding the radix tree node allocation under tree lock constraints. Hence there are a few solutions here. The first is to reinstate GFP_NOFS for the radix tree and add a comment explaining why GFP_NOFS is used. The second is to use memalloc_nofs_save() on the radix tree insert context, which makes it obvious that the radix tree insert runs under GFP_NOFS constraints. The third option is to simply replace the radix tree and it's lock with an xarray which can do memory allocation safely in an insert context. The first is OK, but not really the direction we want to head. The second is my preferred short term solution. The third - converting XFS radix trees to xarray - is the longer term solution. Hence to fix the regression here, we take option 2 as it moves us in the direction we want to head with memory allocation and GFP_NOFS removal. Reported-by: syzbot+8fdff861a781522bda4d@syzkaller.appspotmail.com Reported-by: syzbot+d247769793ec169e4bf9@syzkaller.appspotmail.com Fixes: 94a69db ("xfs: use __GFP_NOLOCKDEP instead of GFP_NOFS") Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
1 parent 215b2bf commit 0c6ca06

File tree

1 file changed

+13
-5
lines changed

1 file changed

+13
-5
lines changed

fs/xfs/xfs_dquot.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,12 @@ xfs_qm_dqget_cache_lookup(
811811
* caller should throw away the dquot and start over. Otherwise, the dquot
812812
* is returned locked (and held by the cache) as if there had been a cache
813813
* hit.
814+
*
815+
* The insert needs to be done under memalloc_nofs context because the radix
816+
* tree can do memory allocation during insert. The qi->qi_tree_lock is taken in
817+
* memory reclaim when freeing unused dquots, so we cannot have the radix tree
818+
* node allocation recursing into filesystem reclaim whilst we hold the
819+
* qi_tree_lock.
814820
*/
815821
static int
816822
xfs_qm_dqget_cache_insert(
@@ -820,25 +826,27 @@ xfs_qm_dqget_cache_insert(
820826
xfs_dqid_t id,
821827
struct xfs_dquot *dqp)
822828
{
829+
unsigned int nofs_flags;
823830
int error;
824831

832+
nofs_flags = memalloc_nofs_save();
825833
mutex_lock(&qi->qi_tree_lock);
826834
error = radix_tree_insert(tree, id, dqp);
827835
if (unlikely(error)) {
828836
/* Duplicate found! Caller must try again. */
829-
mutex_unlock(&qi->qi_tree_lock);
830837
trace_xfs_dqget_dup(dqp);
831-
return error;
838+
goto out_unlock;
832839
}
833840

834841
/* Return a locked dquot to the caller, with a reference taken. */
835842
xfs_dqlock(dqp);
836843
dqp->q_nrefs = 1;
837-
838844
qi->qi_dquots++;
839-
mutex_unlock(&qi->qi_tree_lock);
840845

841-
return 0;
846+
out_unlock:
847+
mutex_unlock(&qi->qi_tree_lock);
848+
memalloc_nofs_restore(nofs_flags);
849+
return error;
842850
}
843851

844852
/* Check our input parameters. */

0 commit comments

Comments
 (0)