diff options
author | Robert Love <robert.w.love@intel.com> | 2013-09-05 03:47:27 -0400 |
---|---|---|
committer | Robert Love <robert.w.love@intel.com> | 2013-10-11 16:25:40 -0400 |
commit | 9d34876f820d55c94bd0b2a2ed3d2e2976cbd997 (patch) | |
tree | ee7dd415c30b57215841214959d00c0470787e51 /drivers/scsi | |
parent | 1c2c1b4fbd413fd814807768d2aba9023722ed76 (diff) |
libfcoe: Make fcoe_sysfs optional / fix fnic NULL exception
fnic doesn't use any of the create/destroy/enable/disable interfaces
either from the (legacy) module paramaters or the (new) fcoe_sysfs
interfaces. When fcoe_sysfs was introduced fnic wasn't changed since
it wasn't using the interfaces. libfcoe incorrectly assumed that that
all of its users were using fcoe_sysfs and when adding and deleting
FCFs would assume the existance of a fcoe_ctlr_device. fnic was not
allocating this structure because it doesn't care about the standard
user interfaces (fnic starts on link only). If/When libfcoe tried to use
the fcoe_ctlr_device's lock for the first time a NULL pointer exception
would be triggered.
Since fnic doesn't care about sysfs or user interfaces, the solution
is to drop libfcoe's assumption that all drivers are using fcoe_sysfs.
This patch accomplishes this by changing some of the structure
relationships.
We need a way to determine when a LLD is using fcoe_sysfs or not and
we can do that by checking for the existance of the fcoe_ctlr_device.
Prior to this patch, it was assumed that the fcoe_ctlr structure was
allocated with the fcoe_ctlr_device and immediately followed it in
memory. To reach the fcoe_ctlr_device we would simply go back in memory
from the fcoe_ctlr to get the fcoe_ctlr_device.
Since fnic doesn't allocate the fcoe_ctlr_device, we cannot keep that
assumption. This patch adds a pointer from the fcoe_ctlr to the
fcoe_ctlr_device. For bnx2fc and fcoe we will continue to allocate the
two structures together, but then we'll set the ctlr->cdev pointer
to point at the fcoe_ctlr_device. fnic will not change and will continue
to allocate the fcoe_ctlr itself, and ctlr->cdev will remain NULL.
When libfcoe adds fcoe_fcf's to the fcoe_ctlr it will check if ctlr->cdev
is set and only if so will it continue to interact with fcoe_sysfs.
Signed-off-by: Robert Love <robert.w.love@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Tested-by: Hiral Patel <hiralpat@cisco.com>
Diffstat (limited to 'drivers/scsi')
-rw-r--r-- | drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 1 | ||||
-rw-r--r-- | drivers/scsi/fcoe/fcoe.c | 1 | ||||
-rw-r--r-- | drivers/scsi/fcoe/fcoe_ctlr.c | 94 |
3 files changed, 65 insertions, 31 deletions
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 69ac55495c1d..27f2cc4b97a5 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c | |||
@@ -1381,6 +1381,7 @@ struct bnx2fc_interface *bnx2fc_interface_create(struct bnx2fc_hba *hba, | |||
1381 | return NULL; | 1381 | return NULL; |
1382 | } | 1382 | } |
1383 | ctlr = fcoe_ctlr_device_priv(ctlr_dev); | 1383 | ctlr = fcoe_ctlr_device_priv(ctlr_dev); |
1384 | ctlr->cdev = ctlr_dev; | ||
1384 | interface = fcoe_ctlr_priv(ctlr); | 1385 | interface = fcoe_ctlr_priv(ctlr); |
1385 | dev_hold(netdev); | 1386 | dev_hold(netdev); |
1386 | kref_init(&interface->kref); | 1387 | kref_init(&interface->kref); |
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c index dff40b2fccbd..8626988e12a5 100644 --- a/drivers/scsi/fcoe/fcoe.c +++ b/drivers/scsi/fcoe/fcoe.c | |||
@@ -408,6 +408,7 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev, | |||
408 | } | 408 | } |
409 | 409 | ||
410 | ctlr = fcoe_ctlr_device_priv(ctlr_dev); | 410 | ctlr = fcoe_ctlr_device_priv(ctlr_dev); |
411 | ctlr->cdev = ctlr_dev; | ||
411 | fcoe = fcoe_ctlr_priv(ctlr); | 412 | fcoe = fcoe_ctlr_priv(ctlr); |
412 | 413 | ||
413 | dev_hold(netdev); | 414 | dev_hold(netdev); |
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c index 692c6535fe75..75efdbc54ef8 100644 --- a/drivers/scsi/fcoe/fcoe_ctlr.c +++ b/drivers/scsi/fcoe/fcoe_ctlr.c | |||
@@ -160,10 +160,16 @@ void fcoe_ctlr_init(struct fcoe_ctlr *fip, enum fip_state mode) | |||
160 | } | 160 | } |
161 | EXPORT_SYMBOL(fcoe_ctlr_init); | 161 | EXPORT_SYMBOL(fcoe_ctlr_init); |
162 | 162 | ||
163 | /** | ||
164 | * fcoe_sysfs_fcf_add() - Add a fcoe_fcf{,_device} to a fcoe_ctlr{,_device} | ||
165 | * @new: The newly discovered FCF | ||
166 | * | ||
167 | * Called with fip->ctlr_mutex held | ||
168 | */ | ||
163 | static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new) | 169 | static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new) |
164 | { | 170 | { |
165 | struct fcoe_ctlr *fip = new->fip; | 171 | struct fcoe_ctlr *fip = new->fip; |
166 | struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip); | 172 | struct fcoe_ctlr_device *ctlr_dev; |
167 | struct fcoe_fcf_device *temp, *fcf_dev; | 173 | struct fcoe_fcf_device *temp, *fcf_dev; |
168 | int rc = -ENOMEM; | 174 | int rc = -ENOMEM; |
169 | 175 | ||
@@ -174,8 +180,6 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new) | |||
174 | if (!temp) | 180 | if (!temp) |
175 | goto out; | 181 | goto out; |
176 | 182 | ||
177 | mutex_lock(&ctlr_dev->lock); | ||
178 | |||
179 | temp->fabric_name = new->fabric_name; | 183 | temp->fabric_name = new->fabric_name; |
180 | temp->switch_name = new->switch_name; | 184 | temp->switch_name = new->switch_name; |
181 | temp->fc_map = new->fc_map; | 185 | temp->fc_map = new->fc_map; |
@@ -185,55 +189,83 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new) | |||
185 | temp->fka_period = new->fka_period; | 189 | temp->fka_period = new->fka_period; |
186 | temp->selected = 0; /* default to unselected */ | 190 | temp->selected = 0; /* default to unselected */ |
187 | 191 | ||
188 | fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp); | ||
189 | if (unlikely(!fcf_dev)) | ||
190 | goto unlock; | ||
191 | |||
192 | /* | 192 | /* |
193 | * The fcoe_sysfs layer can return a CONNECTED fcf that | 193 | * If ctlr_dev doesn't exist then it means we're a libfcoe user |
194 | * has a priv (fcf was never deleted) or a CONNECTED fcf | 194 | * who doesn't use fcoe_syfs and didn't allocate a fcoe_ctlr_device. |
195 | * that doesn't have a priv (fcf was deleted). However, | 195 | * fnic would be an example of a driver with this behavior. In this |
196 | * libfcoe will always delete FCFs before trying to add | 196 | * case we want to add the fcoe_fcf to the fcoe_ctlr list, but we |
197 | * them. This is ensured because both recv_adv and | 197 | * don't want to make sysfs changes. |
198 | * age_fcfs are protected by the the fcoe_ctlr's mutex. | ||
199 | * This means that we should never get a FCF with a | ||
200 | * non-NULL priv pointer. | ||
201 | */ | 198 | */ |
202 | BUG_ON(fcf_dev->priv); | ||
203 | 199 | ||
204 | fcf_dev->priv = new; | 200 | ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip); |
205 | new->fcf_dev = fcf_dev; | 201 | if (ctlr_dev) { |
202 | mutex_lock(&ctlr_dev->lock); | ||
203 | fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp); | ||
204 | if (unlikely(!fcf_dev)) { | ||
205 | rc = -ENOMEM; | ||
206 | goto out; | ||
207 | } | ||
208 | |||
209 | /* | ||
210 | * The fcoe_sysfs layer can return a CONNECTED fcf that | ||
211 | * has a priv (fcf was never deleted) or a CONNECTED fcf | ||
212 | * that doesn't have a priv (fcf was deleted). However, | ||
213 | * libfcoe will always delete FCFs before trying to add | ||
214 | * them. This is ensured because both recv_adv and | ||
215 | * age_fcfs are protected by the the fcoe_ctlr's mutex. | ||
216 | * This means that we should never get a FCF with a | ||
217 | * non-NULL priv pointer. | ||
218 | */ | ||
219 | BUG_ON(fcf_dev->priv); | ||
220 | |||
221 | fcf_dev->priv = new; | ||
222 | new->fcf_dev = fcf_dev; | ||
223 | mutex_unlock(&ctlr_dev->lock); | ||
224 | } | ||
206 | 225 | ||
207 | list_add(&new->list, &fip->fcfs); | 226 | list_add(&new->list, &fip->fcfs); |
208 | fip->fcf_count++; | 227 | fip->fcf_count++; |
209 | rc = 0; | 228 | rc = 0; |
210 | 229 | ||
211 | unlock: | ||
212 | mutex_unlock(&ctlr_dev->lock); | ||
213 | |||
214 | out: | 230 | out: |
215 | kfree(temp); | 231 | kfree(temp); |
216 | return rc; | 232 | return rc; |
217 | } | 233 | } |
218 | 234 | ||
235 | /** | ||
236 | * fcoe_sysfs_fcf_del() - Remove a fcoe_fcf{,_device} to a fcoe_ctlr{,_device} | ||
237 | * @new: The FCF to be removed | ||
238 | * | ||
239 | * Called with fip->ctlr_mutex held | ||
240 | */ | ||
219 | static void fcoe_sysfs_fcf_del(struct fcoe_fcf *new) | 241 | static void fcoe_sysfs_fcf_del(struct fcoe_fcf *new) |
220 | { | 242 | { |
221 | struct fcoe_ctlr *fip = new->fip; | 243 | struct fcoe_ctlr *fip = new->fip; |
222 | struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip); | 244 | struct fcoe_ctlr_device *cdev; |
223 | struct fcoe_fcf_device *fcf_dev; | 245 | struct fcoe_fcf_device *fcf_dev; |
224 | 246 | ||
225 | list_del(&new->list); | 247 | list_del(&new->list); |
226 | fip->fcf_count--; | 248 | fip->fcf_count--; |
227 | 249 | ||
228 | mutex_lock(&ctlr_dev->lock); | 250 | /* |
229 | 251 | * If ctlr_dev doesn't exist then it means we're a libfcoe user | |
230 | fcf_dev = fcoe_fcf_to_fcf_dev(new); | 252 | * who doesn't use fcoe_syfs and didn't allocate a fcoe_ctlr_device |
231 | WARN_ON(!fcf_dev); | 253 | * or a fcoe_fcf_device. |
232 | new->fcf_dev = NULL; | 254 | * |
233 | fcoe_fcf_device_delete(fcf_dev); | 255 | * fnic would be an example of a driver with this behavior. In this |
234 | kfree(new); | 256 | * case we want to remove the fcoe_fcf from the fcoe_ctlr list (above), |
235 | 257 | * but we don't want to make sysfs changes. | |
236 | mutex_unlock(&ctlr_dev->lock); | 258 | */ |
259 | cdev = fcoe_ctlr_to_ctlr_dev(fip); | ||
260 | if (cdev) { | ||
261 | mutex_lock(&cdev->lock); | ||
262 | fcf_dev = fcoe_fcf_to_fcf_dev(new); | ||
263 | WARN_ON(!fcf_dev); | ||
264 | new->fcf_dev = NULL; | ||
265 | fcoe_fcf_device_delete(fcf_dev); | ||
266 | kfree(new); | ||
267 | mutex_unlock(&cdev->lock); | ||
268 | } | ||
237 | } | 269 | } |
238 | 270 | ||
239 | /** | 271 | /** |