]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
xfs: use __GFP_NOLOCKDEP instead of GFP_NOFS
authorDave Chinner <dchinner@redhat.com>
Mon, 22 Apr 2024 17:00:51 +0000 (10:00 -0700)
committerDarrick J. Wong <djwong@kernel.org>
Mon, 3 Jun 2024 18:37:35 +0000 (11:37 -0700)
Source kernel commit: 94a69db2367efcd7e0eeb5d4603340aff1d3c340

In the past we've had problems with lockdep false positives stemming
from inode locking occurring in memory reclaim contexts (e.g. from
superblock shrinkers). Lockdep doesn't know that inodes access from
above memory reclaim cannot be accessed from below memory reclaim
(and vice versa) but there has never been a good solution to solving
this problem with lockdep annotations.

This situation isn't unique to inode locks - buffers are also locked
above and below memory reclaim, and we have to maintain lock
ordering for them - and against inodes - appropriately. IOWs, the
same code paths and locks are taken both above and below memory
reclaim and so we always need to make sure the lock orders are
consistent. We are spared the lockdep problems this might cause
by the fact that semaphores and bit locks aren't covered by lockdep.

In general, this sort of lockdep false positive detection is cause
by code that runs GFP_KERNEL memory allocation with an actively
referenced inode locked. When it is run from a transaction, memory
allocation is automatically GFP_NOFS, so we don't have reclaim
recursion issues. So in the places where we do memory allocation
with inodes locked outside of a transaction, we have explicitly set
them to use GFP_NOFS allocations to prevent lockdep false positives
from being reported if the allocation dips into direct memory
reclaim.

More recently, __GFP_NOLOCKDEP was added to the memory allocation
flags to tell lockdep not to track that particular allocation for
the purposes of reclaim recursion detection. This is a much better
way of preventing false positives - it allows us to use GFP_KERNEL
context outside of transactions, and allows direct memory reclaim to
proceed normally without throwing out false positive deadlock
warnings.

The obvious places that lock inodes and do memory allocation are the
lookup paths and inode extent list initialisation. These occur in
non-transactional GFP_KERNEL contexts, and so can run direct reclaim
and lock inodes.

This patch makes a first path through all the explicit GFP_NOFS
allocations in XFS and converts the obvious ones to GFP_KERNEL |
__GFP_NOLOCKDEP as a first step towards removing explicit GFP_NOFS
allocations from the XFS code.

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>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
libxfs/xfs_ag.c
libxfs/xfs_btree.h
libxfs/xfs_da_btree.c
libxfs/xfs_dir2.c
libxfs/xfs_iext_tree.c
libxfs/xfs_inode_fork.c

index 2ea8d06ca11920c2e73b1c76eb531b5e7a373f03..86024ddfd74a506f2b523227488cdd6f8a48a936 100644 (file)
@@ -387,7 +387,7 @@ xfs_initialize_perag(
                pag->pag_agno = index;
                pag->pag_mount = mp;
 
-               error = radix_tree_preload(GFP_NOFS);
+               error = radix_tree_preload(GFP_KERNEL | __GFP_RETRY_MAYFAIL);
                if (error)
                        goto out_free_pag;
 
index d906324e25c860f7ee89aa56c7267a4fe2e1ed49..75a0e2c8e115b32dba15465b1ec47fa052686d71 100644 (file)
@@ -725,7 +725,9 @@ xfs_btree_alloc_cursor(
 {
        struct xfs_btree_cur    *cur;
 
-       cur = kmem_cache_zalloc(cache, GFP_NOFS | __GFP_NOFAIL);
+       /* BMBT allocations can come through from non-transactional context. */
+       cur = kmem_cache_zalloc(cache,
+                       GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
        cur->bc_tp = tp;
        cur->bc_mp = mp;
        cur->bc_btnum = btnum;
index 91009944922fc56dddd84a8b28aa359e7973cb9e..0fea72f3323d37567ceabb2dbae39b6f1df3915c 100644 (file)
@@ -81,7 +81,8 @@ xfs_da_state_alloc(
 {
        struct xfs_da_state     *state;
 
-       state = kmem_cache_zalloc(xfs_da_state_cache, GFP_NOFS | __GFP_NOFAIL);
+       state = kmem_cache_zalloc(xfs_da_state_cache,
+                       GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
        state->args = args;
        state->mp = args->dp->i_mount;
        return state;
@@ -2515,7 +2516,8 @@ xfs_dabuf_map(
        int                     error = 0, nirecs, i;
 
        if (nfsb > 1)
-               irecs = kzalloc(sizeof(irec) * nfsb, GFP_NOFS | __GFP_NOFAIL);
+               irecs = kzalloc(sizeof(irec) * nfsb,
+                               GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
 
        nirecs = nfsb;
        error = xfs_bmapi_read(dp, bno, nfsb, irecs, &nirecs,
@@ -2529,7 +2531,7 @@ xfs_dabuf_map(
         */
        if (nirecs > 1) {
                map = kzalloc(nirecs * sizeof(struct xfs_buf_map),
-                               GFP_NOFS | __GFP_NOFAIL);
+                               GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
                if (!map) {
                        error = -ENOMEM;
                        goto out_free_irecs;
index c2f0efa063931f275f48e6ebac29ab5ee04306a3..1a2fb999ab08c38370b6bffd8ebbdc31143658b0 100644 (file)
@@ -332,7 +332,8 @@ xfs_dir_cilookup_result(
                                        !(args->op_flags & XFS_DA_OP_CILOOKUP))
                return -EEXIST;
 
-       args->value = kmalloc(len, GFP_NOFS | __GFP_RETRY_MAYFAIL);
+       args->value = kmalloc(len,
+                       GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_RETRY_MAYFAIL);
        if (!args->value)
                return -ENOMEM;
 
@@ -363,15 +364,8 @@ xfs_dir_lookup(
        ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
        XFS_STATS_INC(dp->i_mount, xs_dir_lookup);
 
-       /*
-        * We need to use KM_NOFS here so that lockdep will not throw false
-        * positive deadlock warnings on a non-transactional lookup path. It is
-        * safe to recurse into inode recalim in that case, but lockdep can't
-        * easily be taught about it. Hence KM_NOFS avoids having to add more
-        * lockdep Doing this avoids having to add a bunch of lockdep class
-        * annotations into the reclaim path for the ilock.
-        */
-       args = kzalloc(sizeof(*args), GFP_NOFS | __GFP_NOFAIL);
+       args = kzalloc(sizeof(*args),
+                       GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
        args->geo = dp->i_mount->m_dir_geo;
        args->name = name->name;
        args->namelen = name->len;
index a3bbd9157be3d389a45ef08eb7fd25ac826f6988..cdbb72d63878234d8587df71402bd4c6b97ca5ae 100644 (file)
@@ -394,12 +394,18 @@ xfs_iext_leaf_key(
        return leaf->recs[n].lo & XFS_IEXT_STARTOFF_MASK;
 }
 
+static inline void *
+xfs_iext_alloc_node(
+       int     size)
+{
+       return kzalloc(size, GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
+}
+
 static void
 xfs_iext_grow(
        struct xfs_ifork        *ifp)
 {
-       struct xfs_iext_node    *node = kzalloc(NODE_SIZE,
-                                               GFP_NOFS | __GFP_NOFAIL);
+       struct xfs_iext_node    *node = xfs_iext_alloc_node(NODE_SIZE);
        int                     i;
 
        if (ifp->if_height == 1) {
@@ -455,8 +461,7 @@ xfs_iext_split_node(
        int                     *nr_entries)
 {
        struct xfs_iext_node    *node = *nodep;
-       struct xfs_iext_node    *new = kzalloc(NODE_SIZE,
-                                               GFP_NOFS | __GFP_NOFAIL);
+       struct xfs_iext_node    *new = xfs_iext_alloc_node(NODE_SIZE);
        const int               nr_move = KEYS_PER_NODE / 2;
        int                     nr_keep = nr_move + (KEYS_PER_NODE & 1);
        int                     i = 0;
@@ -544,8 +549,7 @@ xfs_iext_split_leaf(
        int                     *nr_entries)
 {
        struct xfs_iext_leaf    *leaf = cur->leaf;
-       struct xfs_iext_leaf    *new = kzalloc(NODE_SIZE,
-                                               GFP_NOFS | __GFP_NOFAIL);
+       struct xfs_iext_leaf    *new = xfs_iext_alloc_node(NODE_SIZE);
        const int               nr_move = RECS_PER_LEAF / 2;
        int                     nr_keep = nr_move + (RECS_PER_LEAF & 1);
        int                     i;
@@ -586,8 +590,7 @@ xfs_iext_alloc_root(
 {
        ASSERT(ifp->if_bytes == 0);
 
-       ifp->if_data = kzalloc(sizeof(struct xfs_iext_rec),
-                                       GFP_NOFS | __GFP_NOFAIL);
+       ifp->if_data = xfs_iext_alloc_node(sizeof(struct xfs_iext_rec));
        ifp->if_height = 1;
 
        /* now that we have a node step into it */
@@ -607,7 +610,8 @@ xfs_iext_realloc_root(
        if (new_size / sizeof(struct xfs_iext_rec) == RECS_PER_LEAF)
                new_size = NODE_SIZE;
 
-       new = krealloc(ifp->if_data, new_size, GFP_NOFS | __GFP_NOFAIL);
+       new = krealloc(ifp->if_data, new_size,
+                       GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
        memset(new + ifp->if_bytes, 0, new_size - ifp->if_bytes);
        ifp->if_data = new;
        cur->leaf = new;
index 5e0cb488635cf7e5dc3fbcb0ce92bcc06b8afc38..cb1964189f5c00e2263c30fdba1cb94394834ebe 100644 (file)
@@ -48,7 +48,8 @@ xfs_init_local_fork(
                mem_size++;
 
        if (size) {
-               char *new_data = kmalloc(mem_size, GFP_NOFS | __GFP_NOFAIL);
+               char *new_data = kmalloc(mem_size,
+                               GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
 
                memcpy(new_data, data, size);
                if (zero_terminate)
@@ -203,7 +204,8 @@ xfs_iformat_btree(
        }
 
        ifp->if_broot_bytes = size;
-       ifp->if_broot = kmalloc(size, GFP_NOFS | __GFP_NOFAIL);
+       ifp->if_broot = kmalloc(size,
+                               GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
        ASSERT(ifp->if_broot != NULL);
        /*
         * Copy and convert from the on-disk structure
@@ -688,7 +690,7 @@ xfs_ifork_init_cow(
                return;
 
        ip->i_cowfp = kmem_cache_zalloc(xfs_ifork_cache,
-                                      GFP_NOFS | __GFP_NOFAIL);
+                               GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
        ip->i_cowfp->if_format = XFS_DINODE_FMT_EXTENTS;
 }