From 3120885a46f5bf709fbdc5ff5c5c0026f561688b Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Mon, 30 Aug 2010 20:34:17 +0000 Subject: [PATCH] - Eliminate theorethically possible race leading to sending wrong max_sn to initiators - Alignment cleanups git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@2022 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/iscsi.c | 68 +++++++++++++++++++++------------------ iscsi-scst/kernel/iscsi.h | 17 +++++++--- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/iscsi-scst/kernel/iscsi.c b/iscsi-scst/kernel/iscsi.c index d92a84f81..c61c920fa 100644 --- a/iscsi-scst/kernel/iscsi.c +++ b/iscsi-scst/kernel/iscsi.c @@ -343,6 +343,29 @@ static void cmnd_free(struct iscsi_cmnd *cmnd) return; } +static void iscsi_dec_active_cmds(struct iscsi_cmnd *req) +{ + struct iscsi_session *sess = req->conn->session; + + TRACE_DBG("Decrementing active_cmds (req %p, sess %p, " + "new value %d)", req, sess, + atomic_read(&sess->active_cmds)-1); + + EXTRACHECKS_BUG_ON(!req->dec_active_cmds); + + atomic_dec(&sess->active_cmds); + smp_mb__after_atomic_dec(); + req->dec_active_cmds = 0; +#ifdef CONFIG_SCST_EXTRACHECKS + if (unlikely(atomic_read(&sess->active_cmds) < 0)) { + PRINT_CRIT_ERROR("active_cmds < 0 (%d)!!", + atomic_read(&sess->active_cmds)); + sBUG(); + } +#endif + return; +} + /* Might be called under some lock and on SIRQ */ void cmnd_done(struct iscsi_cmnd *cmnd) { @@ -425,20 +448,8 @@ void cmnd_done(struct iscsi_cmnd *cmnd) #endif } - if (cmnd->dec_active_cmnds) { - struct iscsi_session *sess = cmnd->conn->session; - TRACE_DBG("Decrementing active_cmds (cmd %p, sess %p, " - "new value %d)", cmnd, sess, - atomic_read(&sess->active_cmds)-1); - atomic_dec(&sess->active_cmds); -#ifdef CONFIG_SCST_EXTRACHECKS - if (unlikely(atomic_read(&sess->active_cmds) < 0)) { - PRINT_CRIT_ERROR("active_cmds < 0 (%d)!!", - atomic_read(&sess->active_cmds)); - sBUG(); - } -#endif - } + if (unlikely(cmnd->dec_active_cmds)) + iscsi_dec_active_cmds(cmnd); list_for_each_entry_safe(rsp, t, &cmnd->rsp_cmd_list, rsp_cmd_list_entry) { @@ -460,7 +471,7 @@ void cmnd_done(struct iscsi_cmnd *cmnd) #endif } - EXTRACHECKS_BUG_ON(cmnd->dec_active_cmnds); + EXTRACHECKS_BUG_ON(cmnd->dec_active_cmds); if (cmnd == parent->main_rsp) { TRACE_DBG("Finishing main rsp %p (req %p)", cmnd, @@ -601,21 +612,8 @@ static void req_cmnd_pre_release(struct iscsi_cmnd *req) EXTRACHECKS_BUG_ON(req->pending); - if (req->dec_active_cmnds) { - struct iscsi_session *sess = req->conn->session; - TRACE_DBG("Decrementing active_cmds (cmd %p, sess %p, " - "new value %d)", req, sess, - atomic_read(&sess->active_cmds)-1); - atomic_dec(&sess->active_cmds); - req->dec_active_cmnds = 0; -#ifdef CONFIG_SCST_EXTRACHECKS - if (unlikely(atomic_read(&sess->active_cmds) < 0)) { - PRINT_CRIT_ERROR("active_cmds < 0 (%d)!!", - atomic_read(&sess->active_cmds)); - sBUG(); - } -#endif - } + if (unlikely(req->dec_active_cmds)) + iscsi_dec_active_cmds(req); TRACE_EXIT(); return; @@ -1901,7 +1899,7 @@ static int scsi_cmnd_start(struct iscsi_cmnd *req) "new value %d)", req, session, atomic_read(&session->active_cmds)+1); atomic_inc(&session->active_cmds); - req->dec_active_cmnds = 1; + req->dec_active_cmds = 1; scst_cmd = scst_rx_cmd(session->scst_sess, (uint8_t *)&req_hdr->lun, sizeof(req_hdr->lun), @@ -3280,6 +3278,14 @@ static int iscsi_xmit_response(struct scst_cmd *scst_cmd) sBUG(); /* ToDo */ } + /* + * We need to decrement active_cmds before adding any responses into + * the write queue to eliminate a race, when all responses sent + * with wrong MaxCmdSN. + */ + if (likely(req->dec_active_cmds)) + iscsi_dec_active_cmds(req); + if (req->bufflen != 0) { /* * Check above makes sure that is_send_status is set, diff --git a/iscsi-scst/kernel/iscsi.h b/iscsi-scst/kernel/iscsi.h index 9267be740..37d90c148 100644 --- a/iscsi-scst/kernel/iscsi.h +++ b/iscsi-scst/kernel/iscsi.h @@ -173,7 +173,7 @@ struct iscsi_conn { #define ISCSI_CONN_REINSTATING 1 #define ISCSI_CONN_SHUTTINGDOWN 2 - unsigned long conn_aflags __attribute__((aligned(sizeof(long)))); + unsigned long conn_aflags; spinlock_t cmd_list_lock; /* BH lock */ @@ -192,7 +192,10 @@ struct iscsi_conn { struct timer_list rsp_timer; unsigned int rsp_timeout; /* in jiffies */ - /* All 2 protected by iscsi_wr_lock */ + /* + * All 2 protected by iscsi_wr_lock. Modified independently to the + * above field, hence the alignment. + */ unsigned short wr_state __attribute__((aligned(sizeof(long)))); unsigned short wr_space_ready:1; @@ -344,7 +347,7 @@ struct iscsi_cmnd { unsigned int on_write_list:1; unsigned int write_processing_started:1; unsigned int force_cleanup_done:1; - unsigned int dec_active_cmnds:1; + unsigned int dec_active_cmds:1; unsigned int ddigest_checked:1; #ifdef CONFIG_SCST_EXTRACHECKS unsigned int on_rx_digest_list:1; @@ -361,7 +364,7 @@ struct iscsi_cmnd { */ #define ISCSI_CMD_ABORTED 0 #define ISCSI_CMD_PRELIM_COMPLETED 1 - unsigned long prelim_compl_flags __attribute__((aligned(sizeof(long)))); + unsigned long prelim_compl_flags; struct list_head hash_list_entry; @@ -421,7 +424,11 @@ struct iscsi_cmnd { struct iscsi_cmnd *main_rsp; - /* Protected on modify by conn->write_list_lock */ + /* + * Protected on modify by conn->write_list_lock, hence + * modified independently to the above field, hence the + * alignment. + */ int not_processed_rsp_cnt __attribute__((aligned(sizeof(long)))); };