From 038f51cb45812964eedfdc3ac9076a33e0fb4e5b Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Tue, 12 Aug 2008 20:46:32 -0500 Subject: [PATCH 01/23] libfc: use spin_lock_bh for scsi pkt lock fc_fcp.c must lock between a the fcoe recv threads and its timer, so it should be using spin_lock_bh instead of just spin_lock. It looks like this should also be safe to call into send_frame with the scsi pkt lock held using the bh lock call, because the spin_lock_bh code will handle the case where the network layer will also do spin_lock_bh/spin_unlock_bh calls. Note that it looks like other drivers are not going to use fc_fcp.c, so I only take into consideration fcoe's recv threads for this patch and not any other possible drivers. Signed-off-by: Mike Christie --- drivers/scsi/libfc/fc_fcp.c | 42 +++++++++++++++++------------------------- 1 files changed, 17 insertions(+), 25 deletions(-) diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 125cf04..1b56c8c 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -253,17 +253,9 @@ static void fc_fcp_pkt_release(struct fc_fcp_pkt *sp) */ static inline int fc_fcp_lock_pkt(struct fc_fcp_pkt *fsp) { - /* - * TODO mnc: locking is not right. This can be called - * from a timer context so we need to stop bottom halves from the - * thread caller. - * - * It can also be called while sending packets, which can result - * in bh's being enabled and disabled. - */ - spin_lock(&fsp->scsi_pkt_lock); + spin_lock_bh(&fsp->scsi_pkt_lock); if (!fsp->cmd) { - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); FC_DBG("Invalid scsi cmd pointer on fcp packet.\n"); return -EINVAL; } @@ -274,7 +266,7 @@ static inline int fc_fcp_lock_pkt(struct fc_fcp_pkt *fsp) static inline void fc_fcp_unlock_pkt(struct fc_fcp_pkt *fsp) { - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); fc_fcp_pkt_release(fsp); } @@ -1020,10 +1012,10 @@ static int fc_fcp_pkt_abort(struct fc_lport *lp, struct fc_fcp_pkt *fsp) init_completion(&fsp->tm_done); fsp->wait_for_comp = 1; - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); rc = wait_for_completion_timeout(&fsp->tm_done, msecs_to_jiffies(FC_SCSI_TM_TOV)); - spin_lock(&fsp->scsi_pkt_lock); + spin_lock_bh(&fsp->scsi_pkt_lock); if (fsp->seq_ptr) { lp->tt.exch_done(fsp->seq_ptr); @@ -1057,7 +1049,7 @@ static void fc_lun_reset_send(unsigned long data) struct fc_rport *rport; struct fc_rport_libfc_priv *rp; - spin_lock(&fsp->scsi_pkt_lock); + spin_lock_bh(&fsp->scsi_pkt_lock); if (fsp->state & FC_SRB_COMPL) goto unlock; @@ -1088,7 +1080,7 @@ retry: setup_timer(&fsp->timer, fc_lun_reset_send, (unsigned long)fsp); fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV); unlock: - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); } static void fc_fcp_cleanup_lun_reset(struct fc_fcp_pkt *fsp) @@ -1126,13 +1118,13 @@ static int fc_lun_reset(struct fc_lport *lp, struct fc_fcp_pkt *fsp, */ rc = wait_for_completion_timeout(&fsp->tm_done, FC_SCSI_TM_TOV); - spin_lock(&fsp->scsi_pkt_lock); + spin_lock_bh(&fsp->scsi_pkt_lock); fsp->state |= FC_SRB_COMPL; - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); del_timer_sync(&fsp->timer); - spin_lock(&fsp->scsi_pkt_lock); + spin_lock_bh(&fsp->scsi_pkt_lock); if (fsp->seq_ptr) { /* TODO: * if the exch resp function is running and trying to grab @@ -1144,7 +1136,7 @@ static int fc_lun_reset(struct fc_lport *lp, struct fc_fcp_pkt *fsp, fsp->seq_ptr = NULL; } fsp->wait_for_comp = 0; - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); if (!rc) { FC_DBG("lun reset failed\n"); @@ -1167,7 +1159,7 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) { struct fc_fcp_pkt *fsp = arg; - spin_lock(&fsp->scsi_pkt_lock); + spin_lock_bh(&fsp->scsi_pkt_lock); /* * raced with eh timeout handler. * @@ -1176,7 +1168,7 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) */ if ((fsp->state & FC_SRB_COMPL) || !fsp->seq_ptr || !fsp->wait_for_comp) { - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); return; } @@ -1185,7 +1177,7 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) * If there is an error just let it timeout. * scsi-eh will escalate for us. */ - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); return; } @@ -1193,7 +1185,7 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) fsp->seq_ptr = NULL; fsp->lp->tt.exch_done(sp); fc_frame_free(fp); - spin_unlock(&fsp->scsi_pkt_lock); + spin_unlock_bh(&fsp->scsi_pkt_lock); } static void fc_fcp_cleanup_io(struct fc_fcp_pkt *fsp) @@ -1783,9 +1775,9 @@ static void fc_io_compl(struct fc_fcp_pkt *sp) sp->state |= FC_SRB_COMPL; if (!(sp->state & FC_SRB_FCP_PROCESSING_TMO)) { - spin_unlock(&sp->scsi_pkt_lock); + spin_unlock_bh(&sp->scsi_pkt_lock); del_timer_sync(&sp->timer); - spin_lock(&sp->scsi_pkt_lock); + spin_lock_bh(&sp->scsi_pkt_lock); } lp = sp->lp; -- 1.5.4.1