From 1eb68c6f22307abd1e0db95d0b9e017502c4d390 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Mon, 24 Dec 2007 19:22:02 +0000 Subject: [PATCH] - Broken VERIFY commands fixed (hopefully; compile tested only) - Mistyped spin_lock() instead spin_lock_bh() fixed - DEBUG_TM and small logging improvements - Docs update git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@239 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/README | 4 ++++ scst/README | 8 +++++++ scst/src/scst_cdbprobe.h | 8 +++---- scst/src/scst_lib.c | 49 ++++++++++++++++++++++++++-------------- scst/src/scst_priv.h | 4 ++-- scst/src/scst_targ.c | 6 +++-- 6 files changed, 54 insertions(+), 25 deletions(-) diff --git a/iscsi-scst/README b/iscsi-scst/README index 36cc6d199..fdd644353 100644 --- a/iscsi-scst/README +++ b/iscsi-scst/README @@ -111,6 +111,10 @@ writes from VMware on a target disk, which use LVM in the snapshot mode. In this case value like 16 or even 10 depending of your backstorage speed could be more appropriate. +CAUTION: Working of target and initiator on the same host isn't +======== supported. See SCST README file for details. + + Compilation options ------------------- diff --git a/scst/README b/scst/README index 7f1b3d827..838b237ca 100644 --- a/scst/README +++ b/scst/README @@ -95,6 +95,14 @@ IMPORTANT: Without loading appropriate device handler, corresponding devices 'echo "scsi add-single-device A 0 0 B" >/proc/scsi/scsi', where A - is the host number, B - LUN. +IMPORTANT: Working of target and initiator on the same host isn't +========= supported. This is a limitation of the Linux memory/cache + manager, because in this case an OOM deadlock like: system + needs some memory -> it decides to clear some cache -> cache + needs to write on a target exported device -> initiator sends + request to the target -> target needs memory -> problem is + possible. + IMPORTANT: Experience shows that if people work with out of SCST tree target ========= drivers, like target driver for Infiniband or in case if they downloaded and use the released versions of SCST and target diff --git a/scst/src/scst_cdbprobe.h b/scst/src/scst_cdbprobe.h index 315e55114..84aa82e3d 100644 --- a/scst/src/scst_cdbprobe.h +++ b/scst/src/scst_cdbprobe.h @@ -172,7 +172,7 @@ static const struct scst_sdbops scst_scsi_op_table[] = { {0x12, "MMMMMMMMMMMMMMMM", "INQUIRY", SCST_DATA_READ, SCST_SMALL_TIMEOUT, 4, get_trans_len_1}, {0x13, "VOVVVV ", "VERIFY(6)", - SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 2, get_trans_len_3}, + SCST_DATA_NONE, SCST_TRANSFER_LEN_TYPE_FIXED, 2, get_trans_len_3}, {0x14, "VOOVVV ", "RECOVER BUFFERED DATA", SCST_DATA_READ, SCST_TRANSFER_LEN_TYPE_FIXED, 2, get_trans_len_3}, {0x15, "OMOOOOOOOOOOOOOO", "MODE SELECT(6)", @@ -248,7 +248,7 @@ static const struct scst_sdbops scst_scsi_op_table[] = { {0x2E, "O OO O ", "WRITE AND VERIFY(10)", SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 7, get_trans_len_2}, {0x2F, "O OO O ", "VERIFY(10)", - SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 7, get_trans_len_2}, + SCST_DATA_NONE, SCST_TRANSFER_LEN_TYPE_FIXED, 7, get_trans_len_2}, {0x33, "O OO O ", "SET LIMITS(10)", SCST_DATA_NONE, FLAG_NONE, 0, get_trans_len_none}, {0x34, " O ", "READ POSITION", @@ -376,7 +376,7 @@ static const struct scst_sdbops scst_scsi_op_table[] = { {0x8E, "O OO O ", "WRITE AND VERIFY(16)", SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 10, get_trans_len_4}, {0x8F, "O OO O ", "VERIFY(16)", - SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 10, get_trans_len_4}, + SCST_DATA_NONE, SCST_TRANSFER_LEN_TYPE_FIXED, 10, get_trans_len_4}, {0x90, "O OO O ", "PRE-FETCH(16)", SCST_DATA_NONE, FLAG_NONE, 0, get_trans_len_none}, {0x91, "O OO O ", "SYNCHRONIZE CACHE(16)", @@ -440,7 +440,7 @@ static const struct scst_sdbops scst_scsi_op_table[] = { {0xAE, "O OO O ", "WRITE AND VERIFY(12)", SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 6, get_trans_len_4}, {0xAF, "O OO O ", "VERIFY(12)", - SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 6, get_trans_len_4}, + SCST_DATA_NONE, SCST_TRANSFER_LEN_TYPE_FIXED, 6, get_trans_len_4}, /* No need to support at all. {0xB0, " OO O ", "SEARCH DATA HIGH(12)", SCST_DATA_WRITE, FLAG_NONE, 9, get_trans_len_1}, diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 0aa2f4c92..f1143d646 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -1893,8 +1893,7 @@ int scst_sbc_generic_parse(struct scst_cmd *cmd, if ((cmd->cdb[1] & BYTCHK) == 0) { cmd->data_len = cmd->bufflen << get_block_shift(cmd); cmd->bufflen = 0; - cmd->data_direction = SCST_DATA_NONE; - cmd->op_flags &= ~SCST_TRANSFER_LEN_TYPE_FIXED; + goto out; } else cmd->data_len = 0; break; @@ -1911,6 +1910,7 @@ int scst_sbc_generic_parse(struct scst_cmd *cmd, cmd->bufflen = cmd->bufflen << get_block_shift(cmd); } +out: TRACE_DBG("res %d, bufflen %d, data_len %d, direct %d", res, cmd->bufflen, cmd->data_len, cmd->data_direction); @@ -1943,8 +1943,7 @@ int scst_cdrom_generic_parse(struct scst_cmd *cmd, if ((cmd->cdb[1] & BYTCHK) == 0) { cmd->data_len = cmd->bufflen << get_block_shift(cmd); cmd->bufflen = 0; - cmd->data_direction = SCST_DATA_NONE; - cmd->op_flags &= ~SCST_TRANSFER_LEN_TYPE_FIXED; + goto out; } break; default: @@ -1955,8 +1954,9 @@ int scst_cdrom_generic_parse(struct scst_cmd *cmd, if (cmd->op_flags & SCST_TRANSFER_LEN_TYPE_FIXED) cmd->bufflen = cmd->bufflen << get_block_shift(cmd); - TRACE_DBG("res %d bufflen %d direct %d", - res, cmd->bufflen, cmd->data_direction); +out: + TRACE_DBG("res=%d, bufflen=%d, direct=%d", res, cmd->bufflen, + cmd->data_direction); TRACE_EXIT(); return res; @@ -1987,8 +1987,7 @@ int scst_modisk_generic_parse(struct scst_cmd *cmd, if ((cmd->cdb[1] & BYTCHK) == 0) { cmd->data_len = cmd->bufflen << get_block_shift(cmd); cmd->bufflen = 0; - cmd->data_direction = SCST_DATA_NONE; - cmd->op_flags &= ~SCST_TRANSFER_LEN_TYPE_FIXED; + goto out; } break; default: @@ -1999,8 +1998,9 @@ int scst_modisk_generic_parse(struct scst_cmd *cmd, if (cmd->op_flags & SCST_TRANSFER_LEN_TYPE_FIXED) cmd->bufflen = cmd->bufflen << get_block_shift(cmd); - TRACE_DBG("res %d bufflen %d direct %d", - res, cmd->bufflen, cmd->data_direction); +out: + TRACE_DBG("res=%d, bufflen=%d, direct=%d", res, cmd->bufflen, + cmd->data_direction); TRACE_EXIT_RES(res); return res; @@ -2488,7 +2488,7 @@ void scst_check_set_UA(struct scst_tgt_dev *tgt_dev, TRACE_ENTRY(); - spin_lock(&tgt_dev->tgt_dev_lock); + spin_lock_bh(&tgt_dev->tgt_dev_lock); list_for_each_entry(UA_entry_tmp, &tgt_dev->UA_list, UA_list_entry) { @@ -2502,7 +2502,7 @@ void scst_check_set_UA(struct scst_tgt_dev *tgt_dev, if (skip_UA == 0) scst_alloc_set_UA(tgt_dev, sense, sense_len, head); - spin_unlock(&tgt_dev->tgt_dev_lock); + spin_unlock_bh(&tgt_dev->tgt_dev_lock); TRACE_EXIT(); return; @@ -3321,17 +3321,32 @@ void tm_dbg_release_cmd(struct scst_cmd *cmd) spin_unlock_irqrestore(&scst_tm_dbg_lock, flags); } -/* No locks */ -void tm_dbg_task_mgmt(struct scst_tgt_dev *tgt_dev, const char *fn, int force) +/* Might be called under scst_mutex */ +void tm_dbg_task_mgmt(struct scst_device *dev, const char *fn, int force) { unsigned long flags; if (!tm_dbg_flags.tm_dbg_active) goto out; - if ((tgt_dev != NULL) && !test_bit(SCST_TGT_DEV_UNDER_TM_DBG, - &tgt_dev->tgt_dev_flags)) - goto out; + if (dev != NULL) { + struct scst_tgt_dev *tgt_dev; + bool found = 0; + + spin_lock_bh(&dev->dev_lock); + list_for_each_entry(tgt_dev, &dev->dev_tgt_dev_list, + dev_tgt_dev_list_entry) { + if (test_bit(SCST_TGT_DEV_UNDER_TM_DBG, + &tgt_dev->tgt_dev_flags)) { + found = 1; + break; + } + } + spin_unlock_bh(&dev->dev_lock); + + if (!found) + goto out; + } spin_lock_irqsave(&scst_tm_dbg_lock, flags); if ((tm_dbg_state != TM_DBG_STATE_OFFLINE) || force) { diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 257498ed8..1c2fc6dac 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -512,7 +512,7 @@ extern void tm_dbg_deinit_tgt_dev(struct scst_tgt_dev *tgt_dev); extern void tm_dbg_check_released_cmds(void); extern int tm_dbg_check_cmd(struct scst_cmd *cmd); extern void tm_dbg_release_cmd(struct scst_cmd *cmd); -extern void tm_dbg_task_mgmt(struct scst_tgt_dev *tgt_dev, const char *fn, +extern void tm_dbg_task_mgmt(struct scst_device *dev, const char *fn, int force); extern int tm_dbg_is_release(void); #else @@ -525,7 +525,7 @@ static inline int tm_dbg_check_cmd(struct scst_cmd *cmd) return 0; } static inline void tm_dbg_release_cmd(struct scst_cmd *cmd) {} -static inline void tm_dbg_task_mgmt(struct scst_tgt_dev *tgt_dev, const char *fn, +static inline void tm_dbg_task_mgmt(struct scst_device *dev, const char *fn, int force) {} static inline int tm_dbg_is_release(void) { diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index df95061a5..e08e42ca2 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -3729,6 +3729,8 @@ static int scst_target_reset(struct scst_mgmt_cmd *mcmd) } if (!found) list_add_tail(&dev->tm_dev_list_entry, &host_devs); + + tm_dbg_task_mgmt(dev, "TARGET RESET", 0); } /* @@ -3761,7 +3763,6 @@ static int scst_target_reset(struct scst_mgmt_cmd *mcmd) mutex_unlock(&scst_mutex); - tm_dbg_task_mgmt(mcmd->mcmd_tgt_dev, "TARGET RESET", 0); res = scst_set_mcmd_next_state(mcmd); TRACE_EXIT_RES(res); @@ -3802,7 +3803,8 @@ static int scst_lun_reset(struct scst_mgmt_cmd *mcmd) } out_tm_dbg: - tm_dbg_task_mgmt(mcmd->mcmd_tgt_dev, "LUN RESET", 0); + tm_dbg_task_mgmt(mcmd->mcmd_tgt_dev->dev, "LUN RESET", 0); + res = scst_set_mcmd_next_state(mcmd); TRACE_EXIT_RES(res);