diff --git a/scst/include/scst.h b/scst/include/scst.h index 5da726d7d..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 @@ -5106,7 +5104,7 @@ static inline int scst_get_out_buf_count(struct scst_cmd *cmd) return (cmd->out_sg_cnt == 0) ? 1 : cmd->out_sg_cnt; } -int scst_get_buf_full(struct scst_cmd *cmd, uint8_t **buf); +int scst_get_buf_full(struct scst_cmd *cmd, uint8_t **buf, bool always_copy); int scst_get_buf_full_sense(struct scst_cmd *cmd, uint8_t **buf); void scst_put_buf_full(struct scst_cmd *cmd, uint8_t *buf); 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_copy_mgr.c b/scst/src/scst_copy_mgr.c index 344b69e37..e8808a465 100644 --- a/scst/src/scst_copy_mgr.c +++ b/scst/src/scst_copy_mgr.c @@ -2352,7 +2352,7 @@ static void scst_cm_init_inq_finish(struct scst_cmd *cmd) goto out; } - length = scst_get_buf_full(cmd, &buf); + length = scst_get_buf_full(cmd, &buf, false); TRACE_DBG("length %d", length); if (unlikely(length <= 0)) { if (length < 0) diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 7d1acc962..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; @@ -5953,7 +5948,7 @@ static void scst_complete_request_sense(struct scst_cmd *req_cmd) sBUG_ON(orig_cmd == NULL); - len = scst_get_buf_full(req_cmd, &buf); + len = scst_get_buf_full(req_cmd, &buf, false); if (scsi_status_is_good(req_cmd->status) && (len > 0) && scst_sense_valid(buf)) { @@ -9113,12 +9108,13 @@ EXPORT_SYMBOL_GPL(scst_copy_sg); * scst_get_buf_full - return linear buffer for command * @cmd: scst command * @buf: pointer on the resulting pointer + * @always_copy: copy the data buffer even for commands that read data * * If the command's buffer >single page, it vmalloc() the needed area * and copies the buffer there. Returns length of the buffer or negative * error code otherwise. */ -int scst_get_buf_full(struct scst_cmd *cmd, uint8_t **buf) +int scst_get_buf_full(struct scst_cmd *cmd, uint8_t **buf, bool always_copy) { int res = 0; @@ -9153,7 +9149,8 @@ int scst_get_buf_full(struct scst_cmd *cmd, uint8_t **buf) } cmd->sg_buff_vmallocated = 1; - if (scst_cmd_get_data_direction(cmd) == SCST_DATA_WRITE) { + if (scst_cmd_get_data_direction(cmd) == SCST_DATA_WRITE || + always_copy) { uint8_t *buf_ptr; buf_ptr = *buf; @@ -9191,7 +9188,7 @@ int scst_get_buf_full_sense(struct scst_cmd *cmd, uint8_t **buf) TRACE_ENTRY(); - res = scst_get_buf_full(cmd, buf); + res = scst_get_buf_full(cmd, buf, false); if (unlikely(res < 0)) { PRINT_ERROR("scst_get_buf_full() failed: %d", res); if (res == -ENOMEM) @@ -12573,7 +12570,7 @@ int scst_block_generic_dev_done(struct scst_cmd *cmd, int buffer_size, sector_size, sh; uint8_t *buffer; - buffer_size = scst_get_buf_full(cmd, &buffer); + buffer_size = scst_get_buf_full(cmd, &buffer, false); sect_sz_off = opcode == READ_CAPACITY ? 4 : 8; if (buffer_size < sect_sz_off + 4) { scst_put_buf_full(cmd, buffer); @@ -12619,7 +12616,7 @@ int scst_tape_generic_dev_done(struct scst_cmd *cmd, switch (opcode) { case MODE_SENSE: case MODE_SELECT: - buffer_size = scst_get_buf_full(cmd, &buffer); + buffer_size = scst_get_buf_full(cmd, &buffer, false); if (unlikely(buffer_size <= 0)) { if (buffer_size < 0) { PRINT_ERROR("%s: Unable to get the buffer (%d)", 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 818e2d816..c5d0fda31 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -3299,7 +3299,7 @@ next: uint8_t *address; bool err = false; - length = scst_get_buf_full(cmd, &address); + length = scst_get_buf_full(cmd, &address, true); if (length < 0) { PRINT_ERROR("%s", "Unable to get " "MODE_SENSE buffer"); @@ -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,17 +4179,20 @@ 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; + TRACE_DBG("tgt_dev %p found", tgt_dev); - if (likely(tgt_dev->dev->handler != &scst_null_devtype)) { + if (likely(dev->handler != &scst_null_devtype)) { cmd->cmd_threads = tgt_dev->active_cmd_threads; cmd->tgt_dev = tgt_dev; cmd->cur_order_data = tgt_dev->curr_order_data; - cmd->dev = tgt_dev->dev; - cmd->devt = tgt_dev->dev->handler; + cmd->dev = dev; + cmd->devt = dev->handler; + cmd->owns_refcnt = true; + percpu_ref_get(&dev->refcnt); res = 0; } else { @@ -4199,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, @@ -4338,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); diff --git a/scst/src/scst_tg.c b/scst/src/scst_tg.c index b316ac184..e392a3c3f 100644 --- a/scst/src/scst_tg.c +++ b/scst/src/scst_tg.c @@ -1839,7 +1839,7 @@ int scst_tg_set_group_info(struct scst_cmd *cmd) TRACE_ENTRY(); - len = scst_get_buf_full(cmd, &buf); + len = scst_get_buf_full(cmd, &buf, false); if (len < 0) { PRINT_ERROR("scst_get_buf_full() failed: %d", len); res = len; diff --git a/scstadmin/init.d/scst b/scstadmin/init.d/scst index 40d6c54c3..3c6acdaa1 100755 --- a/scstadmin/init.d/scst +++ b/scstadmin/init.d/scst @@ -192,10 +192,9 @@ unload_scst() { # seconds. unload_kmod isert_scst 90 || echo "Unloading isert_scst failed" - for m in scst_disk scst_raid scst_tape scst_user scst_changer \ - scst_cdrom scst_vdisk scst_modisk scst_processor \ - isert_scst iscsi_scst fcst ib_srpt qla2xxx_scst qla2x00tgt \ - scst_local scst; do + for m in isert_scst iscsi_scst fcst ib_srpt qla2xxx_scst qla2x00tgt \ + scst_local scst_disk scst_raid scst_tape scst_user scst_changer \ + scst_cdrom scst_vdisk scst_modisk scst_processor scst; do unload_kmod "$m" 30 done reverse_list=""