NFSv4: Clean up nfs_client_return_marked_delegations()
authorTrond Myklebust <trond.myklebust@hammerspace.com>
Thu, 27 Feb 2020 13:29:02 +0000 (08:29 -0500)
committerTrond Myklebust <trond.myklebust@hammerspace.com>
Mon, 16 Mar 2020 12:34:30 +0000 (08:34 -0400)
Convert it to use the nfs_client_for_each_server() helper, and
make it more efficient by skipping delegations for inodes we
know are in the process of being freed. Also improve the efficiency
of the cursor by skipping delegations that are being freed.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
fs/nfs/delegation.c

index 509b723..19f66d3 100644 (file)
@@ -563,21 +563,11 @@ static bool nfs_delegation_need_return(struct nfs_delegation *delegation)
        return ret;
 }
 
-/**
- * nfs_client_return_marked_delegations - return previously marked delegations
- * @clp: nfs_client to process
- *
- * Note that this function is designed to be called by the state
- * manager thread. For this reason, it cannot flush the dirty data,
- * since that could deadlock in case of a state recovery error.
- *
- * Returns zero on success, or a negative errno value.
- */
-int nfs_client_return_marked_delegations(struct nfs_client *clp)
+static int nfs_server_return_marked_delegations(struct nfs_server *server,
+               void __always_unused *data)
 {
        struct nfs_delegation *delegation;
        struct nfs_delegation *prev;
-       struct nfs_server *server;
        struct inode *inode;
        struct inode *place_holder = NULL;
        struct nfs_delegation *place_holder_deleg = NULL;
@@ -587,78 +577,79 @@ restart:
        /*
         * To avoid quadratic looping we hold a reference
         * to an inode place_holder.  Each time we restart, we
-        * list nfs_servers from the server of that inode, and
-        * delegation in the server from the delegations of that
-        * inode.
+        * list delegation in the server from the delegations
+        * of that inode.
         * prev is an RCU-protected pointer to a delegation which
         * wasn't marked for return and might be a good choice for
         * the next place_holder.
         */
-       rcu_read_lock();
        prev = NULL;
+       delegation = NULL;
+       rcu_read_lock();
        if (place_holder)
-               server = NFS_SERVER(place_holder);
-       else
-               server = list_entry_rcu(clp->cl_superblocks.next,
-                                       struct nfs_server, client_link);
-       list_for_each_entry_from_rcu(server, &clp->cl_superblocks, client_link) {
-               delegation = NULL;
-               if (place_holder && server == NFS_SERVER(place_holder))
-                       delegation = rcu_dereference(NFS_I(place_holder)->delegation);
-               if (!delegation || delegation != place_holder_deleg)
-                       delegation = list_entry_rcu(server->delegations.next,
-                                                   struct nfs_delegation, super_list);
-               list_for_each_entry_from_rcu(delegation, &server->delegations, super_list) {
-                       struct inode *to_put = NULL;
-
-                       if (!nfs_delegation_need_return(delegation)) {
+               delegation = rcu_dereference(NFS_I(place_holder)->delegation);
+       if (!delegation || delegation != place_holder_deleg)
+               delegation = list_entry_rcu(server->delegations.next,
+                                           struct nfs_delegation, super_list);
+       list_for_each_entry_from_rcu(delegation, &server->delegations, super_list) {
+               struct inode *to_put = NULL;
+
+               if (test_bit(NFS_DELEGATION_INODE_FREEING, &delegation->flags))
+                       continue;
+               if (!nfs_delegation_need_return(delegation)) {
+                       if (nfs4_is_valid_delegation(delegation, 0))
                                prev = delegation;
-                               continue;
-                       }
-                       if (!nfs_sb_active(server->super))
-                               break; /* continue in outer loop */
-
-                       if (prev) {
-                               struct inode *tmp;
+                       continue;
+               }
 
-                               tmp = nfs_delegation_grab_inode(prev);
-                               if (tmp) {
-                                       to_put = place_holder;
-                                       place_holder = tmp;
-                                       place_holder_deleg = prev;
-                               }
+               if (prev) {
+                       struct inode *tmp = nfs_delegation_grab_inode(prev);
+                       if (tmp) {
+                               to_put = place_holder;
+                               place_holder = tmp;
+                               place_holder_deleg = prev;
                        }
+               }
 
-                       inode = nfs_delegation_grab_inode(delegation);
-                       if (inode == NULL) {
-                               rcu_read_unlock();
-                               if (to_put)
-                                       iput(to_put);
-                               nfs_sb_deactive(server->super);
-                               goto restart;
-                       }
-                       delegation = nfs_start_delegation_return_locked(NFS_I(inode));
+               inode = nfs_delegation_grab_inode(delegation);
+               if (inode == NULL) {
                        rcu_read_unlock();
+                       iput(to_put);
+                       goto restart;
+               }
+               delegation = nfs_start_delegation_return_locked(NFS_I(inode));
+               rcu_read_unlock();
 
-                       if (to_put)
-                               iput(to_put);
+               iput(to_put);
 
-                       err = nfs_end_delegation_return(inode, delegation, 0);
-                       iput(inode);
-                       nfs_sb_deactive(server->super);
-                       cond_resched();
-                       if (!err)
-                               goto restart;
-                       set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state);
-                       if (place_holder)
-                               iput(place_holder);
-                       return err;
-               }
+               err = nfs_end_delegation_return(inode, delegation, 0);
+               iput(inode);
+               cond_resched();
+               if (!err)
+                       goto restart;
+               set_bit(NFS4CLNT_DELEGRETURN, &server->nfs_client->cl_state);
+               goto out;
        }
        rcu_read_unlock();
-       if (place_holder)
-               iput(place_holder);
-       return 0;
+out:
+       iput(place_holder);
+       return err;
+}
+
+/**
+ * nfs_client_return_marked_delegations - return previously marked delegations
+ * @clp: nfs_client to process
+ *
+ * Note that this function is designed to be called by the state
+ * manager thread. For this reason, it cannot flush the dirty data,
+ * since that could deadlock in case of a state recovery error.
+ *
+ * Returns zero on success, or a negative errno value.
+ */
+int nfs_client_return_marked_delegations(struct nfs_client *clp)
+{
+       return nfs_client_for_each_server(clp,
+                       nfs_server_return_marked_delegations, NULL);
 }
 
 /**