Commit 6ff8147d authored by Stefan Richter's avatar Stefan Richter

firewire: sbp2: remove obsolete reference counting

Since commit 0278ccd9 "firewire: sbp2:
fix panic after rmmod with slow targets", the lifetime of an sbp2_target
instance does no longer extent past the return of sbp2_remove().
Therefore it is no longer necessary to call fw_unit_get/put() and
fw_device_get/put() in sbp2_probe/remove().

Furthermore, said commit also ensures that lu->work is not going to be
executed or requeued at a time when the sbp2_target is no longer in use.
Hence there is no need for sbp2_target reference counting for lu->work.

Other concurrent contexts:

  - Processes which access the sysfs of the SCSI host device or of one
    of its subdevices are safe because these interfaces are all removed
    by scsi_remove_device/host() in sbp2_release_target().

  - SBP-2 command block ORB transactions are finished when
    scsi_remove_device() in sbp2_release_target() returns.

  - SBP-2 management ORB transactions are finished when
    cancel_delayed_work_sync(&lu->work) before sbp2_release_target()
    returns.
Signed-off-by: default avatarStefan Richter <stefanr@s5r6.in-berlin.de>
parent f39aa30d
...@@ -159,7 +159,6 @@ struct sbp2_logical_unit { ...@@ -159,7 +159,6 @@ struct sbp2_logical_unit {
* and one struct Scsi_Host per sbp2_target. * and one struct Scsi_Host per sbp2_target.
*/ */
struct sbp2_target { struct sbp2_target {
struct kref kref;
struct fw_unit *unit; struct fw_unit *unit;
const char *bus_id; const char *bus_id;
struct list_head lu_list; struct list_head lu_list;
...@@ -772,9 +771,8 @@ static int sbp2_lun2int(u16 lun) ...@@ -772,9 +771,8 @@ static int sbp2_lun2int(u16 lun)
return scsilun_to_int(&eight_bytes_lun); return scsilun_to_int(&eight_bytes_lun);
} }
static void sbp2_release_target(struct kref *kref) static void sbp2_release_target(struct sbp2_target *tgt)
{ {
struct sbp2_target *tgt = container_of(kref, struct sbp2_target, kref);
struct sbp2_logical_unit *lu, *next; struct sbp2_logical_unit *lu, *next;
struct Scsi_Host *shost = struct Scsi_Host *shost =
container_of((void *)tgt, struct Scsi_Host, hostdata[0]); container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
...@@ -811,30 +809,12 @@ static void sbp2_release_target(struct kref *kref) ...@@ -811,30 +809,12 @@ static void sbp2_release_target(struct kref *kref)
scsi_remove_host(shost); scsi_remove_host(shost);
fw_notify("released %s, target %d:0:0\n", tgt->bus_id, shost->host_no); fw_notify("released %s, target %d:0:0\n", tgt->bus_id, shost->host_no);
fw_unit_put(tgt->unit);
scsi_host_put(shost); scsi_host_put(shost);
fw_device_put(device);
}
static void sbp2_target_get(struct sbp2_target *tgt)
{
kref_get(&tgt->kref);
}
static void sbp2_target_put(struct sbp2_target *tgt)
{
kref_put(&tgt->kref, sbp2_release_target);
} }
/*
* Always get the target's kref when scheduling work on one its units.
* Each workqueue job is responsible to call sbp2_target_put() upon return.
*/
static void sbp2_queue_work(struct sbp2_logical_unit *lu, unsigned long delay) static void sbp2_queue_work(struct sbp2_logical_unit *lu, unsigned long delay)
{ {
sbp2_target_get(lu->tgt); queue_delayed_work(fw_workqueue, &lu->work, delay);
if (!queue_delayed_work(fw_workqueue, &lu->work, delay))
sbp2_target_put(lu->tgt);
} }
/* /*
...@@ -877,7 +857,7 @@ static void sbp2_login(struct work_struct *work) ...@@ -877,7 +857,7 @@ static void sbp2_login(struct work_struct *work)
int generation, node_id, local_node_id; int generation, node_id, local_node_id;
if (fw_device_is_shutdown(device)) if (fw_device_is_shutdown(device))
goto out; return;
generation = device->generation; generation = device->generation;
smp_rmb(); /* node IDs must not be older than generation */ smp_rmb(); /* node IDs must not be older than generation */
...@@ -899,7 +879,7 @@ static void sbp2_login(struct work_struct *work) ...@@ -899,7 +879,7 @@ static void sbp2_login(struct work_struct *work)
/* Let any waiting I/O fail from now on. */ /* Let any waiting I/O fail from now on. */
sbp2_unblock(lu->tgt); sbp2_unblock(lu->tgt);
} }
goto out; return;
} }
tgt->node_id = node_id; tgt->node_id = node_id;
...@@ -925,7 +905,8 @@ static void sbp2_login(struct work_struct *work) ...@@ -925,7 +905,8 @@ static void sbp2_login(struct work_struct *work)
if (lu->has_sdev) { if (lu->has_sdev) {
sbp2_cancel_orbs(lu); sbp2_cancel_orbs(lu);
sbp2_conditionally_unblock(lu); sbp2_conditionally_unblock(lu);
goto out;
return;
} }
if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY) if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY)
...@@ -957,7 +938,8 @@ static void sbp2_login(struct work_struct *work) ...@@ -957,7 +938,8 @@ static void sbp2_login(struct work_struct *work)
lu->has_sdev = true; lu->has_sdev = true;
scsi_device_put(sdev); scsi_device_put(sdev);
sbp2_allow_block(lu); sbp2_allow_block(lu);
goto out;
return;
out_logout_login: out_logout_login:
smp_rmb(); /* generation may have changed */ smp_rmb(); /* generation may have changed */
...@@ -971,8 +953,6 @@ static void sbp2_login(struct work_struct *work) ...@@ -971,8 +953,6 @@ static void sbp2_login(struct work_struct *work)
* lu->work already. Reset the work from reconnect to login. * lu->work already. Reset the work from reconnect to login.
*/ */
PREPARE_DELAYED_WORK(&lu->work, sbp2_login); PREPARE_DELAYED_WORK(&lu->work, sbp2_login);
out:
sbp2_target_put(tgt);
} }
static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry) static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry)
...@@ -1141,7 +1121,6 @@ static int sbp2_probe(struct device *dev) ...@@ -1141,7 +1121,6 @@ static int sbp2_probe(struct device *dev)
tgt = (struct sbp2_target *)shost->hostdata; tgt = (struct sbp2_target *)shost->hostdata;
dev_set_drvdata(&unit->device, tgt); dev_set_drvdata(&unit->device, tgt);
tgt->unit = unit; tgt->unit = unit;
kref_init(&tgt->kref);
INIT_LIST_HEAD(&tgt->lu_list); INIT_LIST_HEAD(&tgt->lu_list);
tgt->bus_id = dev_name(&unit->device); tgt->bus_id = dev_name(&unit->device);
tgt->guid = (u64)device->config_rom[3] << 32 | device->config_rom[4]; tgt->guid = (u64)device->config_rom[3] << 32 | device->config_rom[4];
...@@ -1154,9 +1133,6 @@ static int sbp2_probe(struct device *dev) ...@@ -1154,9 +1133,6 @@ static int sbp2_probe(struct device *dev)
if (scsi_add_host(shost, &unit->device) < 0) if (scsi_add_host(shost, &unit->device) < 0)
goto fail_shost_put; goto fail_shost_put;
fw_device_get(device);
fw_unit_get(unit);
/* implicit directory ID */ /* implicit directory ID */
tgt->directory_id = ((unit->directory - device->config_rom) * 4 tgt->directory_id = ((unit->directory - device->config_rom) * 4
+ CSR_CONFIG_ROM) & 0xffffff; + CSR_CONFIG_ROM) & 0xffffff;
...@@ -1166,7 +1142,7 @@ static int sbp2_probe(struct device *dev) ...@@ -1166,7 +1142,7 @@ static int sbp2_probe(struct device *dev)
if (sbp2_scan_unit_dir(tgt, unit->directory, &model, if (sbp2_scan_unit_dir(tgt, unit->directory, &model,
&firmware_revision) < 0) &firmware_revision) < 0)
goto fail_tgt_put; goto fail_release_target;
sbp2_clamp_management_orb_timeout(tgt); sbp2_clamp_management_orb_timeout(tgt);
sbp2_init_workarounds(tgt, model, firmware_revision); sbp2_init_workarounds(tgt, model, firmware_revision);
...@@ -1183,10 +1159,11 @@ static int sbp2_probe(struct device *dev) ...@@ -1183,10 +1159,11 @@ static int sbp2_probe(struct device *dev)
/* Do the login in a workqueue so we can easily reschedule retries. */ /* Do the login in a workqueue so we can easily reschedule retries. */
list_for_each_entry(lu, &tgt->lu_list, link) list_for_each_entry(lu, &tgt->lu_list, link)
sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5)); sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5));
return 0; return 0;
fail_tgt_put: fail_release_target:
sbp2_target_put(tgt); sbp2_release_target(tgt);
return -ENOMEM; return -ENOMEM;
fail_shost_put: fail_shost_put:
...@@ -1203,7 +1180,8 @@ static int sbp2_remove(struct device *dev) ...@@ -1203,7 +1180,8 @@ static int sbp2_remove(struct device *dev)
list_for_each_entry(lu, &tgt->lu_list, link) list_for_each_entry(lu, &tgt->lu_list, link)
cancel_delayed_work_sync(&lu->work); cancel_delayed_work_sync(&lu->work);
sbp2_target_put(tgt); sbp2_release_target(tgt);
return 0; return 0;
} }
...@@ -1216,7 +1194,7 @@ static void sbp2_reconnect(struct work_struct *work) ...@@ -1216,7 +1194,7 @@ static void sbp2_reconnect(struct work_struct *work)
int generation, node_id, local_node_id; int generation, node_id, local_node_id;
if (fw_device_is_shutdown(device)) if (fw_device_is_shutdown(device))
goto out; return;
generation = device->generation; generation = device->generation;
smp_rmb(); /* node IDs must not be older than generation */ smp_rmb(); /* node IDs must not be older than generation */
...@@ -1241,7 +1219,8 @@ static void sbp2_reconnect(struct work_struct *work) ...@@ -1241,7 +1219,8 @@ static void sbp2_reconnect(struct work_struct *work)
PREPARE_DELAYED_WORK(&lu->work, sbp2_login); PREPARE_DELAYED_WORK(&lu->work, sbp2_login);
} }
sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5)); sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5));
goto out;
return;
} }
tgt->node_id = node_id; tgt->node_id = node_id;
...@@ -1255,8 +1234,6 @@ static void sbp2_reconnect(struct work_struct *work) ...@@ -1255,8 +1234,6 @@ static void sbp2_reconnect(struct work_struct *work)
sbp2_agent_reset(lu); sbp2_agent_reset(lu);
sbp2_cancel_orbs(lu); sbp2_cancel_orbs(lu);
sbp2_conditionally_unblock(lu); sbp2_conditionally_unblock(lu);
out:
sbp2_target_put(tgt);
} }
static void sbp2_update(struct fw_unit *unit) static void sbp2_update(struct fw_unit *unit)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment