--- /dev/null
+From: Robert Love <robert.w.love@intel.com>
+Subject: libfc: Ensure correct device_put/get usage (round 2)
+References:
+
+Reference counting was barely used and where used
+it was incorrect. This patch creates a few simple
+policies.
+
+When the rport->dev [e.g. struct device] is initialized
+it starts with a refcnt of 1. Whenever we're using the
+rport we will increment the count. When we logoff we
+should decrement the count to 0 and the 'release'
+function will be called. The FC transport provides the
+release function for real rports and libfc provides it
+for rogue rports. When we switch from a rogue to real
+rport we'll decrement the refcnt on the rogue rport
+and increment it for the real rport, after we've created
+it.
+
+Any externally initiated action on an rport (login,
+logoff) will not require the caller to increment and
+decrement the refcnt.
+
+For rport_login(), the rport will have just been created
+and therefore no other thread would be able to access
+this object.
+
+For rport_logoff(), the rport will have been removed
+from the list of rports and therefore no other thread
+would be able to lookup() this rport.
+
+This patch removes the get_device() from the rport_lookup
+function. These are the places where it is called and why
+we don't need a reference.
+
+fc_disc_recv_rscn_req() - called for single port RSCNs
+ the disc mutex is held and
+ ensures that no other thread
+ will find this rport.
+
+fc_disc_new_target() - Same. The rport cannot be looked up
+ so no other thread can free the rport.
+ This code looks buggy though, we
+ shouldn't be calling rport_login() on
+ a 'real' rport, which we could do.
+
+fc_disc_single() - Same. disc mutex protects the list.
+
+fc_lport_recv_req() - Similar, but this time the lport lock
+ ensures that no incoming requests are
+ processed until the current request
+ for an rport has returned.
+
+When the rport layer needs to send a request it will
+increment the count so that the EM can be confident that
+the rport is present when making the callback. If
+fc_remote_port_delete() is called before the response
+callback, which is often the case for LOGO commands, the
+refcnt will still have a value of 1 becuase we grabbed the
+lock before the ctels_send() is called. The exchange would
+have been removed and so the callback will be called with
+an error code. After processing the error code we'll
+decrement the refcnt for the last time and the rport will
+be free'd.
+
+Since point-to-point mode is not working this patch
+does not consider point-to-point.
+
+Signed-off-by: Robert Love <robert.w.love@intel.com>
+Acked-by: Bernhard Walle <bwalle@suse.de>
+---
+
+ drivers/scsi/libfc/fc_disc.c | 5 +----
+ drivers/scsi/libfc/fc_lport.c | 5 ++---
+ drivers/scsi/libfc/fc_rport.c | 21 ++++++++++++++-------
+ 3 files changed, 17 insertions(+), 14 deletions(-)
+
+
+--- a/drivers/scsi/libfc/fc_disc.c
++++ b/drivers/scsi/libfc/fc_disc.c
+@@ -81,7 +81,6 @@ struct fc_rport *fc_disc_lookup_rport(co
+ if (rport->port_id == port_id) {
+ disc_found = 1;
+ found = rport;
+- get_device(&found->dev);
+ break;
+ }
+ }
+@@ -767,10 +766,8 @@ static void fc_disc_single(struct fc_dis
+ goto out;
+
+ rport = lport->tt.rport_lookup(lport, dp->ids.port_id);
+- if (rport) {
++ if (rport)
+ fc_disc_del_target(disc, rport);
+- put_device(&rport->dev); /* hold from lookup */
+- }
+
+ new_rport = fc_rport_rogue_create(dp);
+ if (new_rport) {
+--- a/drivers/scsi/libfc/fc_lport.c
++++ b/drivers/scsi/libfc/fc_lport.c
+@@ -908,10 +908,9 @@ static void fc_lport_recv_req(struct fc_
+ d_id = ntoh24(fh->fh_d_id);
+
+ rport = lport->tt.rport_lookup(lport, s_id);
+- if (rport) {
++ if (rport)
+ lport->tt.rport_recv_req(sp, fp, rport);
+- put_device(&rport->dev); /* hold from lookup */
+- } else {
++ else {
+ rjt_data.fp = NULL;
+ rjt_data.reason = ELS_RJT_UNAB;
+ rjt_data.explan = ELS_EXPL_NONE;
+--- a/drivers/scsi/libfc/fc_rport.c
++++ b/drivers/scsi/libfc/fc_rport.c
+@@ -111,16 +111,11 @@ struct fc_rport *fc_rport_rogue_create(s
+ rport->roles = dp->ids.roles;
+ rport->maxframe_size = FC_MIN_MAX_PAYLOAD;
+ /*
+- * init the device, so other code can manipulate the rport as if
+- * it came from the fc class. We also do an extra get because
+- * libfc will free this rport instead of relying on the normal
+- * refcounting.
+- *
+ * Note: all this libfc rogue rport code will be removed for
+ * upstream so it fine that this is really ugly and hacky right now.
+ */
+ device_initialize(&rport->dev);
+- get_device(&rport->dev);
++ rport->dev.release = fc_rport_rogue_destroy; // XXX: bwalle
+
+ mutex_init(&rdata->rp_mutex);
+ rdata->local_port = dp->lp;
+@@ -402,9 +397,9 @@ static void fc_rport_timeout(struct work
+ case RPORT_ST_NONE:
+ break;
+ }
+- put_device(&rport->dev);
+
+ mutex_unlock(&rdata->rp_mutex);
++ put_device(&rport->dev);
+ }
+
+ /**
+@@ -531,6 +526,7 @@ out:
+ fc_frame_free(fp);
+ err:
+ mutex_unlock(&rdata->rp_mutex);
++ put_device(&rport->dev);
+ }
+
+ /**
+@@ -562,6 +558,8 @@ static void fc_rport_enter_plogi(struct
+ if (!lport->tt.elsct_send(lport, rport, fp, ELS_PLOGI,
+ fc_rport_plogi_resp, rport, lport->e_d_tov))
+ fc_rport_error(rport, fp);
++ else
++ get_device(&rport->dev);
+ }
+
+ /**
+@@ -631,6 +629,7 @@ out:
+ fc_frame_free(fp);
+ err:
+ mutex_unlock(&rdata->rp_mutex);
++ put_device(&rport->dev);
+ }
+
+ /**
+@@ -679,6 +678,7 @@ out:
+ fc_frame_free(fp);
+ err:
+ mutex_unlock(&rdata->rp_mutex);
++ put_device(&rport->dev);
+ }
+
+ /**
+@@ -712,6 +712,8 @@ static void fc_rport_enter_prli(struct f
+ if (!lport->tt.elsct_send(lport, rport, fp, ELS_PRLI,
+ fc_rport_prli_resp, rport, lport->e_d_tov))
+ fc_rport_error(rport, fp);
++ else
++ get_device(&rport->dev);
+ }
+
+ /**
+@@ -777,6 +779,7 @@ out:
+ fc_frame_free(fp);
+ err:
+ mutex_unlock(&rdata->rp_mutex);
++ put_device(&rport->dev);
+ }
+
+ /**
+@@ -806,6 +809,8 @@ static void fc_rport_enter_rtv(struct fc
+ if (!lport->tt.elsct_send(lport, rport, fp, ELS_RTV,
+ fc_rport_rtv_resp, rport, lport->e_d_tov))
+ fc_rport_error(rport, fp);
++ else
++ get_device(&rport->dev);
+ }
+
+ /**
+@@ -835,6 +840,8 @@ static void fc_rport_enter_logo(struct f
+ if (!lport->tt.elsct_send(lport, rport, fp, ELS_LOGO,
+ fc_rport_logo_resp, rport, lport->e_d_tov))
+ fc_rport_error(rport, fp);
++ else
++ get_device(&rport->dev);
+ }
+
+
projects / people / pmueller / ipfire-2.x.git / blobdiff