nfsd: fix callback restarts

Checking the rpc_client pointer is not a reliable way to detect
backchannel changes: cl_cb_client is changed only after shutting down
the rpc client, so the condition cl_cb_client = tk_client will always be
true.

Check the RPC_TASK_KILLED flag instead, and rewrite the code to avoid
the buggy cl_callbacks list and fix the lifetime rules due to double
calls of the ->prepare callback operations method for this retry case.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index cd58b7cd3f1adde6c5b2f62ac8f2c4e0316b07c9..4c993aaf1e6e54ef946434689bdcfa0613605e3c 100644 (file)
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -879,13 +879,6 @@ static void nfsd4_cb_prepare(struct rpc_task *task, void *calldata)
                if (!nfsd41_cb_get_slot(clp, task))
                        return;
        }
-       spin_lock(&clp->cl_lock);
-       if (list_empty(&cb->cb_per_client)) {
-               /* This is the first call, not a restart */
-               cb->cb_done = false;
-               list_add(&cb->cb_per_client, &clp->cl_callbacks);
-       }
-       spin_unlock(&clp->cl_lock);
        rpc_call_start(task);
 }
 
@@ -907,16 +900,21 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
                        clp->cl_cb_session->se_cb_seq_nr);
        }
 
-       if (clp->cl_cb_client != task->tk_client) {
-               /* We're shutting down or changing cl_cb_client; leave
-                * it to nfsd4_process_cb_update to restart the call if
-                * necessary. */
+       /*
+        * If the backchannel connection was shut down while this
+        * task was queued, we need to resubmit it after setting up
+        * a new backchannel connection.
+        *
+        * Note that if we lost our callback connection permanently
+        * the submission code will error out, so we don't need to
+        * handle that case here.
+        */
+       if (task->tk_flags & RPC_TASK_KILLED) {
+               task->tk_status = 0;
+               cb->cb_need_restart = true;
                return;
        }
 
-       if (cb->cb_done)
-               return;
-
        if (cb->cb_status) {
                WARN_ON_ONCE(task->tk_status);
                task->tk_status = cb->cb_status;
@@ -936,21 +934,17 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
        default:
                BUG();
        }
-       cb->cb_done = true;
 }
 
 static void nfsd4_cb_release(void *calldata)
 {
        struct nfsd4_callback *cb = calldata;
-       struct nfs4_client *clp = cb->cb_clp;
-
-       if (cb->cb_done) {
-               spin_lock(&clp->cl_lock);
-               list_del(&cb->cb_per_client);
-               spin_unlock(&clp->cl_lock);
 
+       if (cb->cb_need_restart)
+               nfsd4_run_cb(cb);
+       else
                cb->cb_ops->release(cb);
-       }
+
 }
 
 static const struct rpc_call_ops nfsd4_cb_ops = {
@@ -1045,9 +1039,6 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb)
                nfsd4_mark_cb_down(clp, err);
                return;
        }
-       /* Yay, the callback channel's back! Restart any callbacks: */
-       list_for_each_entry(cb, &clp->cl_callbacks, cb_per_client)
-               queue_work(callback_wq, &cb->cb_work);
 }
 
 static void
@@ -1058,8 +1049,12 @@ nfsd4_run_cb_work(struct work_struct *work)
        struct nfs4_client *clp = cb->cb_clp;
        struct rpc_clnt *clnt;
 
-       if (cb->cb_ops && cb->cb_ops->prepare)
-               cb->cb_ops->prepare(cb);
+       if (cb->cb_need_restart) {
+               cb->cb_need_restart = false;
+       } else {
+               if (cb->cb_ops && cb->cb_ops->prepare)
+                       cb->cb_ops->prepare(cb);
+       }
 
        if (clp->cl_flags & NFSD4_CLIENT_CB_FLAG_MASK)
                nfsd4_process_cb_update(cb);
@@ -1085,9 +1080,8 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
        cb->cb_msg.rpc_resp = cb;
        cb->cb_ops = ops;
        INIT_WORK(&cb->cb_work, nfsd4_run_cb_work);
-       INIT_LIST_HEAD(&cb->cb_per_client);
        cb->cb_status = 0;
-       cb->cb_done = true;
+       cb->cb_need_restart = false;
 }
 
 void nfsd4_run_cb(struct nfsd4_callback *cb)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 66067a291267884346703d686f85664106300e6d..039f9c8a95e84289c7296e9b78973efbea9d6e5a 100644 (file)
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1626,7 +1626,6 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
        INIT_LIST_HEAD(&clp->cl_openowners);
        INIT_LIST_HEAD(&clp->cl_delegations);
        INIT_LIST_HEAD(&clp->cl_lru);
-       INIT_LIST_HEAD(&clp->cl_callbacks);
        INIT_LIST_HEAD(&clp->cl_revoked);
 #ifdef CONFIG_NFSD_PNFS
        INIT_LIST_HEAD(&clp->cl_lo_states);

diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index e791985a7318cc6ecd45cb8a780735a7be124cc3..dbc4f85a500825a2c80b21a9a92b52ff1631b8fb 100644 (file)
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -63,13 +63,12 @@ typedef struct {
 
 struct nfsd4_callback {
        struct nfs4_client *cb_clp;
-       struct list_head cb_per_client;
        u32 cb_minorversion;
        struct rpc_message cb_msg;
        struct nfsd4_callback_ops *cb_ops;
        struct work_struct cb_work;
        int cb_status;
-       bool cb_done;
+       bool cb_need_restart;
 };
 
 struct nfsd4_callback_ops {
@@ -334,7 +333,6 @@ struct nfs4_client {
        int                     cl_cb_state;
        struct nfsd4_callback   cl_cb_null;
        struct nfsd4_session    *cl_cb_session;
-       struct list_head        cl_callbacks; /* list of in-progress callbacks */
 
        /* for all client information that callback code might need: */
        spinlock_t              cl_lock;