nvmet-fcloop: refactor fcloop_nport_alloc and track lport

The checks for a valid input values are mixed with the logic to insert a
newly allocated nport. Refactor the function so that first the checks
are done.

This allows to untangle the setup steps into a more linear form which
reduces the complexity of the functions.

Also start tracking lport when a lport is assigned to a nport. This
ensures, that the lport is not going away as long it is still referenced
by a nport.

Signed-off-by: Daniel Wagner <wagi@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index 7763b8bf098deedb822d703375e63b125cf83103..a576ad9e1dead78ee8d1995f06fd71109aaabdab 100644 (file)
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -1028,6 +1028,9 @@ fcloop_nport_put(struct fcloop_nport *nport)
        list_del(&nport->nport_list);
        spin_unlock_irqrestore(&fcloop_lock, flags);
 
+       if (nport->lport)
+               fcloop_lport_put(nport->lport);
+
        kfree(nport);
 }
 
@@ -1234,6 +1237,25 @@ fcloop_nport_lookup(u64 node_name, u64 port_name)
        return nport;
 }
 
+static struct fcloop_lport *
+__fcloop_lport_lookup(u64 node_name, u64 port_name)
+{
+       struct fcloop_lport *lport;
+
+       list_for_each_entry(lport, &fcloop_lports, lport_list) {
+               if (lport->localport->node_name != node_name ||
+                   lport->localport->port_name != port_name)
+                       continue;
+
+               if (fcloop_lport_get(lport))
+                       return lport;
+
+               break;
+       }
+
+       return NULL;
+}
+
 static ssize_t
 fcloop_delete_local_port(struct device *dev, struct device_attribute *attr,
                const char *buf, size_t count)
@@ -1272,8 +1294,8 @@ fcloop_delete_local_port(struct device *dev, struct device_attribute *attr,
 static struct fcloop_nport *
 fcloop_alloc_nport(const char *buf, size_t count, bool remoteport)
 {
-       struct fcloop_nport *newnport, *nport = NULL;
-       struct fcloop_lport *tmplport, *lport = NULL;
+       struct fcloop_nport *newnport, *nport;
+       struct fcloop_lport *lport;
        struct fcloop_ctrl_options *opts;
        unsigned long flags;
        u32 opts_mask = (remoteport) ? RPORT_OPTS : TGTPORT_OPTS;
@@ -1288,10 +1310,8 @@ fcloop_alloc_nport(const char *buf, size_t count, bool remoteport)
                goto out_free_opts;
 
        /* everything there ? */
-       if ((opts->mask & opts_mask) != opts_mask) {
-               ret = -EINVAL;
+       if ((opts->mask & opts_mask) != opts_mask)
                goto out_free_opts;
-       }
 
        newnport = kzalloc(sizeof(*newnport), GFP_KERNEL);
        if (!newnport)
@@ -1307,60 +1327,61 @@ fcloop_alloc_nport(const char *buf, size_t count, bool remoteport)
        refcount_set(&newnport->ref, 1);
 
        spin_lock_irqsave(&fcloop_lock, flags);
-
-       list_for_each_entry(tmplport, &fcloop_lports, lport_list) {
-               if (tmplport->localport->node_name == opts->wwnn &&
-                   tmplport->localport->port_name == opts->wwpn)
-                       goto out_invalid_opts;
-
-               if (tmplport->localport->node_name == opts->lpwwnn &&
-                   tmplport->localport->port_name == opts->lpwwpn)
-                       lport = tmplport;
+       lport = __fcloop_lport_lookup(opts->wwnn, opts->wwpn);
+       if (lport) {
+               /* invalid configuration */
+               fcloop_lport_put(lport);
+               goto out_free_newnport;
        }
 
        if (remoteport) {
-               if (!lport)
-                       goto out_invalid_opts;
-               newnport->lport = lport;
-       }
-
-       list_for_each_entry(nport, &fcloop_nports, nport_list) {
-               if (nport->node_name == opts->wwnn &&
-                   nport->port_name == opts->wwpn) {
-                       if ((remoteport && nport->rport) ||
-                           (!remoteport && nport->tport)) {
-                               nport = NULL;
-                               goto out_invalid_opts;
-                       }
-
-                       fcloop_nport_get(nport);
-
-                       spin_unlock_irqrestore(&fcloop_lock, flags);
-
-                       if (remoteport)
-                               nport->lport = lport;
-                       if (opts->mask & NVMF_OPT_ROLES)
-                               nport->port_role = opts->roles;
-                       if (opts->mask & NVMF_OPT_FCADDR)
-                               nport->port_id = opts->fcaddr;
+               lport = __fcloop_lport_lookup(opts->lpwwnn, opts->lpwwpn);
+               if (!lport) {
+                       /* invalid configuration */
                        goto out_free_newnport;
                }
        }
 
-       list_add_tail(&newnport->nport_list, &fcloop_nports);
+       nport = __fcloop_nport_lookup(opts->wwnn, opts->wwpn);
+       if (nport) {
+               if ((remoteport && nport->rport) ||
+                   (!remoteport && nport->tport)) {
+                       /* invalid configuration */
+                       goto out_put_nport;
+               }
 
+               /* found existing nport, discard the new nport */
+               kfree(newnport);
+       } else {
+               list_add_tail(&newnport->nport_list, &fcloop_nports);
+               nport = newnport;
+       }
+
+       if (opts->mask & NVMF_OPT_ROLES)
+               nport->port_role = opts->roles;
+       if (opts->mask & NVMF_OPT_FCADDR)
+               nport->port_id = opts->fcaddr;
+       if (lport) {
+               if (!nport->lport)
+                       nport->lport = lport;
+               else
+                       fcloop_lport_put(lport);
+       }
        spin_unlock_irqrestore(&fcloop_lock, flags);
 
        kfree(opts);
-       return newnport;
+       return nport;
 
-out_invalid_opts:
-       spin_unlock_irqrestore(&fcloop_lock, flags);
+out_put_nport:
+       if (lport)
+               fcloop_lport_put(lport);
+       fcloop_nport_put(nport);
 out_free_newnport:
+       spin_unlock_irqrestore(&fcloop_lock, flags);
        kfree(newnport);
 out_free_opts:
        kfree(opts);
-       return nport;
+       return NULL;
 }
 
 static ssize_t