diff --git a/scst/include/scst.h b/scst/include/scst.h index c1893eb02..bc55275b5 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -2819,8 +2819,6 @@ struct scst_device { /* Triggered when refcnt drops to zero. */ struct completion *remove_completion; - struct work_struct free_work; - /* * Maximum count of uncompleted commands that an initiator could * queue on this device. Then it will start getting TASK QUEUE FULL diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index ef7713b27..9c58f8902 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -1321,8 +1321,6 @@ next: if (res) goto out; - percpu_ref_get(&dev->refcnt); - out: TRACE_EXIT(); return res; @@ -1347,7 +1345,6 @@ static void vdisk_detach(struct scst_device *dev) /* virt_dev will be freed by the caller */ dev->dh_priv = NULL; virt_dev->dev = NULL; - percpu_ref_put(&dev->refcnt); TRACE_EXIT(); return; diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 2643dc013..856b580c8 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -4213,11 +4213,8 @@ static int scst_dif_none_type1(struct scst_cmd *cmd); #endif /* Called from thread context and hence may sleep. */ -static void scst_free_device(struct work_struct *work) +void scst_free_device(struct scst_device *dev) { - struct scst_device *dev = container_of(work, typeof(*dev), - free_work); - EXTRACHECKS_BUG_ON(dev->dev_scsi_atomic_cmd_active != 0); EXTRACHECKS_BUG_ON(!list_empty(&dev->dev_exec_cmd_list)); @@ -4252,7 +4249,6 @@ static void scst_release_device(struct percpu_ref *ref) if (c) complete(c); - schedule_work(&dev->free_work); } int scst_alloc_device(gfp_t gfp_mask, int nodeid, struct scst_device **out_dev) @@ -4273,7 +4269,6 @@ int scst_alloc_device(gfp_t gfp_mask, int nodeid, struct scst_device **out_dev) memset(dev, 0, sizeof(*dev)); dev->handler = &scst_null_devtype; - INIT_WORK(&dev->free_work, scst_free_device); res = percpu_ref_init(&dev->refcnt, scst_release_device, 0, GFP_KERNEL); if (res < 0) goto free_dev; diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index b6c37ee38..da00e9945 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -1130,6 +1130,8 @@ out_del_unlocked: out_free_dev: percpu_ref_kill(&dev->refcnt); + scst_free_device(dev); + out_unlock: mutex_unlock(&scst_mutex); goto out; @@ -1173,8 +1175,6 @@ static void scst_unregister_device(struct scsi_device *scsidp) scst_dg_dev_remove_by_dev(dev); - scst_assign_dev_handler(dev, &scst_null_devtype); - list_for_each_entry_safe(acg_dev, aa, &dev->dev_acg_dev_list, dev_acg_dev_list_entry) { scst_acg_del_lun(acg_dev->acg, acg_dev->lun, true); @@ -1184,8 +1184,6 @@ static void scst_unregister_device(struct scsi_device *scsidp) mutex_unlock(&scst_mutex); - scst_dev_sysfs_del(dev); - PRINT_INFO("Detached from scsi%d, channel %d, id %d, lun %lld, type %d", scsidp->host->host_no, scsidp->channel, scsidp->id, (u64)scsidp->lun, scsidp->type); @@ -1195,6 +1193,14 @@ static void scst_unregister_device(struct scsi_device *scsidp) wait_for_completion(&c); + mutex_lock(&scst_mutex); + scst_assign_dev_handler(dev, &scst_null_devtype); + mutex_unlock(&scst_mutex); + + scst_dev_sysfs_del(dev); + + scst_free_device(dev); + out: TRACE_EXIT(); return; @@ -1399,6 +1405,7 @@ out_free_dev: if (sysfs_del) scst_dev_sysfs_del(dev); percpu_ref_kill(&dev->refcnt); + scst_free_device(dev); goto out; out_unlock: @@ -1444,8 +1451,6 @@ void scst_unregister_virtual_device(int id, scst_dg_dev_remove_by_dev(dev); - scst_assign_dev_handler(dev, &scst_null_devtype); - list_for_each_entry_safe(acg_dev, aa, &dev->dev_acg_dev_list, dev_acg_dev_list_entry) { scst_acg_del_lun(acg_dev->acg, acg_dev->lun, true); @@ -1455,8 +1460,6 @@ void scst_unregister_virtual_device(int id, mutex_unlock(&scst_mutex); - scst_dev_sysfs_del(dev); - PRINT_INFO("Detached from virtual device %s (id %d)", dev->virt_name, dev->virt_id); @@ -1468,6 +1471,14 @@ void scst_unregister_virtual_device(int id, wait_for_completion(&c); + mutex_lock(&scst_mutex); + scst_assign_dev_handler(dev, &scst_null_devtype); + mutex_unlock(&scst_mutex); + + scst_dev_sysfs_del(dev); + + scst_free_device(dev); + out: TRACE_EXIT(); return; @@ -1922,7 +1933,13 @@ out_err: goto out; } -/* The activity supposed to be suspended and scst_mutex held */ +/* + * The caller must hold scst_mutex. No commands must be in progress for @dev. + * There are two ways to achieve this: either make sure no device handler is + * assigned before this function is called or remove all associated LUNs and + * wait until the commands associated with these LUNs have finished before this + * function is called. + */ int scst_assign_dev_handler(struct scst_device *dev, struct scst_dev_type *handler) { @@ -1932,6 +1949,8 @@ int scst_assign_dev_handler(struct scst_device *dev, TRACE_ENTRY(); + lockdep_assert_held(&scst_mutex); + sBUG_ON(handler == NULL); if (dev->handler == handler) diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 47483eec9..a17ac3960 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -370,6 +370,7 @@ int scst_alloc_tgt(struct scst_tgt_template *tgtt, struct scst_tgt **tgt); void scst_free_tgt(struct scst_tgt *tgt); int scst_alloc_device(gfp_t gfp_mask, int nodeid, struct scst_device **out_dev); +void scst_free_device(struct scst_device *dev); bool scst_device_is_exported(struct scst_device *dev); int scst_alloc_add_acg(struct scst_tgt *tgt, const char *acg_name, diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index d01b2bcc2..c5d0fda31 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -3674,6 +3674,7 @@ static int scst_pre_xmit_response1(struct scst_cmd *cmd) * latency, so we should decrement them after cmd completed. */ smp_mb__before_atomic_dec(); + WARN_ON_ONCE(!cmd->owns_refcnt); cmd->owns_refcnt = false; atomic_dec(&cmd->tgt_dev->tgt_dev_cmd_count); percpu_ref_put(&cmd->dev->refcnt); @@ -4178,7 +4179,6 @@ static int scst_translate_lun(struct scst_cmd *cmd) tgt_dev = scst_lookup_tgt_dev(cmd->sess, cmd->lun); if (tgt_dev) atomic_inc(&tgt_dev->tgt_dev_cmd_count); - rcu_read_unlock(); if (tgt_dev) { struct scst_device *dev = tgt_dev->dev; @@ -4191,6 +4191,8 @@ static int scst_translate_lun(struct scst_cmd *cmd) cmd->cur_order_data = tgt_dev->curr_order_data; cmd->dev = dev; cmd->devt = dev->handler; + cmd->owns_refcnt = true; + percpu_ref_get(&dev->refcnt); res = 0; } else { @@ -4201,6 +4203,7 @@ static int scst_translate_lun(struct scst_cmd *cmd) atomic_dec(&tgt_dev->tgt_dev_cmd_count); } } + rcu_read_unlock(); if (unlikely(res != 0)) { if (!nul_dev) { TRACE(TRACE_MINOR, @@ -4340,8 +4343,6 @@ static int __scst_init_cmd(struct scst_cmd *cmd) failure = true; } - cmd->owns_refcnt = true; - percpu_ref_get(&dev->refcnt); #ifdef CONFIG_SCST_PER_DEVICE_CMD_COUNT_LIMIT atomic_inc(&dev->dev_cmd_count); cnt = atomic_read(&dev->dev_cmd_count);