NFSv4/pNFS: Don't call _nfs4_pnfs_v3_ds_connect multiple times
authorTrond Myklebust <trond.myklebust@hammerspace.com>
Sat, 3 Jul 2021 18:34:20 +0000 (14:34 -0400)
committerTrond Myklebust <trond.myklebust@hammerspace.com>
Thu, 8 Jul 2021 18:03:26 +0000 (14:03 -0400)
After we grab the lock in nfs4_pnfs_ds_connect(), there is no check for
whether or not ds->ds_clp has already been initialised, so we can end up
adding the same transports multiple times.

Fixes: fc821d59209d ("pnfs/NFSv4.1: Add multipath capabilities to pNFS flexfiles servers over NFSv3")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
fs/nfs/pnfs_nfs.c

index 49d3389..1c2c0d0 100644 (file)
@@ -805,19 +805,16 @@ out:
 }
 EXPORT_SYMBOL_GPL(nfs4_pnfs_ds_add);
 
-static void nfs4_wait_ds_connect(struct nfs4_pnfs_ds *ds)
+static int nfs4_wait_ds_connect(struct nfs4_pnfs_ds *ds)
 {
        might_sleep();
-       wait_on_bit(&ds->ds_state, NFS4DS_CONNECTING,
-                       TASK_KILLABLE);
+       return wait_on_bit(&ds->ds_state, NFS4DS_CONNECTING, TASK_KILLABLE);
 }
 
 static void nfs4_clear_ds_conn_bit(struct nfs4_pnfs_ds *ds)
 {
        smp_mb__before_atomic();
-       clear_bit(NFS4DS_CONNECTING, &ds->ds_state);
-       smp_mb__after_atomic();
-       wake_up_bit(&ds->ds_state, NFS4DS_CONNECTING);
+       clear_and_wake_up_bit(NFS4DS_CONNECTING, &ds->ds_state);
 }
 
 static struct nfs_client *(*get_v3_ds_connect)(
@@ -993,30 +990,33 @@ int nfs4_pnfs_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds,
 {
        int err;
 
-again:
-       err = 0;
-       if (test_and_set_bit(NFS4DS_CONNECTING, &ds->ds_state) == 0) {
-               if (version == 3) {
-                       err = _nfs4_pnfs_v3_ds_connect(mds_srv, ds, timeo,
-                                                      retrans);
-               } else if (version == 4) {
-                       err = _nfs4_pnfs_v4_ds_connect(mds_srv, ds, timeo,
-                                                      retrans, minor_version);
-               } else {
-                       dprintk("%s: unsupported DS version %d\n", __func__,
-                               version);
-                       err = -EPROTONOSUPPORT;
-               }
+       do {
+               err = nfs4_wait_ds_connect(ds);
+               if (err || ds->ds_clp)
+                       goto out;
+               if (nfs4_test_deviceid_unavailable(devid))
+                       return -ENODEV;
+       } while (test_and_set_bit(NFS4DS_CONNECTING, &ds->ds_state) != 0);
 
-               nfs4_clear_ds_conn_bit(ds);
-       } else {
-               nfs4_wait_ds_connect(ds);
+       if (ds->ds_clp)
+               goto connect_done;
 
-               /* what was waited on didn't connect AND didn't mark unavail */
-               if (!ds->ds_clp && !nfs4_test_deviceid_unavailable(devid))
-                       goto again;
+       switch (version) {
+       case 3:
+               err = _nfs4_pnfs_v3_ds_connect(mds_srv, ds, timeo, retrans);
+               break;
+       case 4:
+               err = _nfs4_pnfs_v4_ds_connect(mds_srv, ds, timeo, retrans,
+                                              minor_version);
+               break;
+       default:
+               dprintk("%s: unsupported DS version %d\n", __func__, version);
+               err = -EPROTONOSUPPORT;
        }
 
+connect_done:
+       nfs4_clear_ds_conn_bit(ds);
+out:
        /*
         * At this point the ds->ds_clp should be ready, but it might have
         * hit an error.