1 From: Robert Love <robert.w.love@intel.com>
2 Subject: libfc: Ensure correct device_put/get usage (round 2)
5 Reference counting was barely used and where used
6 it was incorrect. This patch creates a few simple
9 When the rport->dev [e.g. struct device] is initialized
10 it starts with a refcnt of 1. Whenever we're using the
11 rport we will increment the count. When we logoff we
12 should decrement the count to 0 and the 'release'
13 function will be called. The FC transport provides the
14 release function for real rports and libfc provides it
15 for rogue rports. When we switch from a rogue to real
16 rport we'll decrement the refcnt on the rogue rport
17 and increment it for the real rport, after we've created
20 Any externally initiated action on an rport (login,
21 logoff) will not require the caller to increment and
24 For rport_login(), the rport will have just been created
25 and therefore no other thread would be able to access
28 For rport_logoff(), the rport will have been removed
29 from the list of rports and therefore no other thread
30 would be able to lookup() this rport.
32 This patch removes the get_device() from the rport_lookup
33 function. These are the places where it is called and why
34 we don't need a reference.
36 fc_disc_recv_rscn_req() - called for single port RSCNs
37 the disc mutex is held and
38 ensures that no other thread
41 fc_disc_new_target() - Same. The rport cannot be looked up
42 so no other thread can free the rport.
43 This code looks buggy though, we
44 shouldn't be calling rport_login() on
45 a 'real' rport, which we could do.
47 fc_disc_single() - Same. disc mutex protects the list.
49 fc_lport_recv_req() - Similar, but this time the lport lock
50 ensures that no incoming requests are
51 processed until the current request
52 for an rport has returned.
54 When the rport layer needs to send a request it will
55 increment the count so that the EM can be confident that
56 the rport is present when making the callback. If
57 fc_remote_port_delete() is called before the response
58 callback, which is often the case for LOGO commands, the
59 refcnt will still have a value of 1 becuase we grabbed the
60 lock before the ctels_send() is called. The exchange would
61 have been removed and so the callback will be called with
62 an error code. After processing the error code we'll
63 decrement the refcnt for the last time and the rport will
66 Since point-to-point mode is not working this patch
67 does not consider point-to-point.
69 Signed-off-by: Robert Love <robert.w.love@intel.com>
70 Acked-by: Bernhard Walle <bwalle@suse.de>
73 drivers/scsi/libfc/fc_disc.c | 5 +----
74 drivers/scsi/libfc/fc_lport.c | 5 ++---
75 drivers/scsi/libfc/fc_rport.c | 21 ++++++++++++++-------
76 3 files changed, 17 insertions(+), 14 deletions(-)
79 --- a/drivers/scsi/libfc/fc_disc.c
80 +++ b/drivers/scsi/libfc/fc_disc.c
81 @@ -81,7 +81,6 @@ struct fc_rport *fc_disc_lookup_rport(co
82 if (rport->port_id == port_id) {
85 - get_device(&found->dev);
89 @@ -767,10 +766,8 @@ static void fc_disc_single(struct fc_dis
92 rport = lport->tt.rport_lookup(lport, dp->ids.port_id);
95 fc_disc_del_target(disc, rport);
96 - put_device(&rport->dev); /* hold from lookup */
99 new_rport = fc_rport_rogue_create(dp);
101 --- a/drivers/scsi/libfc/fc_lport.c
102 +++ b/drivers/scsi/libfc/fc_lport.c
103 @@ -908,10 +908,9 @@ static void fc_lport_recv_req(struct fc_
104 d_id = ntoh24(fh->fh_d_id);
106 rport = lport->tt.rport_lookup(lport, s_id);
109 lport->tt.rport_recv_req(sp, fp, rport);
110 - put_device(&rport->dev); /* hold from lookup */
114 rjt_data.reason = ELS_RJT_UNAB;
115 rjt_data.explan = ELS_EXPL_NONE;
116 --- a/drivers/scsi/libfc/fc_rport.c
117 +++ b/drivers/scsi/libfc/fc_rport.c
118 @@ -111,16 +111,11 @@ struct fc_rport *fc_rport_rogue_create(s
119 rport->roles = dp->ids.roles;
120 rport->maxframe_size = FC_MIN_MAX_PAYLOAD;
122 - * init the device, so other code can manipulate the rport as if
123 - * it came from the fc class. We also do an extra get because
124 - * libfc will free this rport instead of relying on the normal
127 * Note: all this libfc rogue rport code will be removed for
128 * upstream so it fine that this is really ugly and hacky right now.
130 device_initialize(&rport->dev);
131 - get_device(&rport->dev);
132 + rport->dev.release = fc_rport_rogue_destroy; // XXX: bwalle
134 mutex_init(&rdata->rp_mutex);
135 rdata->local_port = dp->lp;
136 @@ -402,9 +397,9 @@ static void fc_rport_timeout(struct work
140 - put_device(&rport->dev);
142 mutex_unlock(&rdata->rp_mutex);
143 + put_device(&rport->dev);
147 @@ -531,6 +526,7 @@ out:
150 mutex_unlock(&rdata->rp_mutex);
151 + put_device(&rport->dev);
155 @@ -562,6 +558,8 @@ static void fc_rport_enter_plogi(struct
156 if (!lport->tt.elsct_send(lport, rport, fp, ELS_PLOGI,
157 fc_rport_plogi_resp, rport, lport->e_d_tov))
158 fc_rport_error(rport, fp);
160 + get_device(&rport->dev);
164 @@ -631,6 +629,7 @@ out:
167 mutex_unlock(&rdata->rp_mutex);
168 + put_device(&rport->dev);
172 @@ -679,6 +678,7 @@ out:
175 mutex_unlock(&rdata->rp_mutex);
176 + put_device(&rport->dev);
180 @@ -712,6 +712,8 @@ static void fc_rport_enter_prli(struct f
181 if (!lport->tt.elsct_send(lport, rport, fp, ELS_PRLI,
182 fc_rport_prli_resp, rport, lport->e_d_tov))
183 fc_rport_error(rport, fp);
185 + get_device(&rport->dev);
189 @@ -777,6 +779,7 @@ out:
192 mutex_unlock(&rdata->rp_mutex);
193 + put_device(&rport->dev);
197 @@ -806,6 +809,8 @@ static void fc_rport_enter_rtv(struct fc
198 if (!lport->tt.elsct_send(lport, rport, fp, ELS_RTV,
199 fc_rport_rtv_resp, rport, lport->e_d_tov))
200 fc_rport_error(rport, fp);
202 + get_device(&rport->dev);
206 @@ -835,6 +840,8 @@ static void fc_rport_enter_logo(struct f
207 if (!lport->tt.elsct_send(lport, rport, fp, ELS_LOGO,
208 fc_rport_logo_resp, rport, lport->e_d_tov))
209 fc_rport_error(rport, fp);
211 + get_device(&rport->dev);