From: Mike Christie <[EMAIL PROTECTED]> This patch removes the shared tag map. It is only being used to loop over running commands which is better suited with a list of commands. This patch does not fix the issue with the original outstanding array use and with the shared tag patch, where the resp function can be running while the fc_io_compl is freeing pkt. This will be fixed in a different patch.
This patch does fix several issues with the timer usage in fc_fcp.c in the same code path I changed including: - Use setup_timer and mod_timer. - Don't call del_timer_sync with a spin_lock held. - del_timer_sync does not handle timer functions that rearm themselves. The user must add some synchrinization for this. - The lun reset code could set a timer if the frame allocation fails, but it was not deleting it. This patch was made over my lun reset fixes: http://www.open-fcoe.org/pipermail/devel/2008-July/000441.html Signed-off-by: Mike Christie <[EMAIL PROTECTED]> --- drivers/scsi/fcoe/fcoe_if.c | 1 + drivers/scsi/libfc/fc_fcp.c | 228 +++++++++++++++++++++++------------------- 2 files changed, 126 insertions(+), 103 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe_if.c b/drivers/scsi/fcoe/fcoe_if.c index 359c6a2..406d0c5 100644 --- a/drivers/scsi/fcoe/fcoe_if.c +++ b/drivers/scsi/fcoe/fcoe_if.c @@ -125,6 +125,7 @@ static struct scsi_host_template fcoe_driver_template = { .eh_host_reset_handler = fc_eh_host_reset, .slave_alloc = fc_slave_alloc, .change_queue_depth = fc_change_queue_depth, + .change_queue_type = fc_change_queue_type, .this_id = -1, .cmd_per_lun = 32, .can_queue = FC_MAX_OUTSTANDING_COMMANDS, diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 267e278..3f66c54 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -43,12 +43,13 @@ int fc_fcp_debug; static struct kmem_cache *scsi_pkt_cachep; /* SRB state definitions */ -#define FC_SRB_FREE 0 /* cmd is free */ -#define FC_SRB_CMD_SENT (1 << 0) /* cmd has been sent */ -#define FC_SRB_RCV_STATUS (1 << 1) /* response has arrived */ -#define FC_SRB_ABORT_PENDING (1 << 2) /* cmd abort sent to device */ -#define FC_SRB_ABORTED (1 << 3) /* abort acknowleged */ -#define FC_SRB_DISCONTIG (1 << 4) /* non-sequential data recvd */ +#define FC_SRB_FREE 0 /* cmd is free */ +#define FC_SRB_CMD_SENT (1 << 0) /* cmd has been sent */ +#define FC_SRB_RCV_STATUS (1 << 1) /* response has arrived */ +#define FC_SRB_ABORT_PENDING (1 << 2) /* cmd abort sent to device */ +#define FC_SRB_ABORTED (1 << 3) /* abort acknowleged */ +#define FC_SRB_DISCONTIG (1 << 4) /* non-sequential data recvd */ +#define FC_SRB_COMPL (1 << 5) /* fc_io_compl has been run */ #define FC_SRB_READ (1 << 1) #define FC_SRB_WRITE (1 << 0) @@ -69,7 +70,10 @@ struct fc_fcp_pkt { /* * SCSI I/O related stuff */ - struct scsi_cmnd *cmd; /* scsi command pointer */ + struct scsi_cmnd *cmd; /* scsi command pointer. set/clear + * under host lock */ + struct list_head list; /* tracks queued commands. access under + * host lock */ /* * timeout related stuff */ @@ -124,6 +128,7 @@ struct fc_fcp_pkt { struct fc_fcp_internal { mempool_t *scsi_pkt_pool; + struct list_head scsi_pkt_queue; }; #define fc_get_scsi_internal(x) ((struct fc_fcp_internal *)(x)->scsi_priv) @@ -142,7 +147,7 @@ static void fc_abort_internal(struct fc_fcp_pkt *); static void fc_timeout_error(struct fc_fcp_pkt *); static void fc_fcp_retry_cmd(struct fc_fcp_pkt *); static int fc_fcp_send_cmd(struct fc_fcp_pkt *); -static void fc_fcp_timeout(struct fc_fcp_pkt *); +static void fc_fcp_timeout(unsigned long data); static void fc_fcp_rec(struct fc_fcp_pkt *); static void fc_fcp_rec_error(struct fc_fcp_pkt *, struct fc_frame *); static void fc_fcp_rec_resp(struct fc_seq *, struct fc_frame *, void *); @@ -198,6 +203,7 @@ static struct fc_fcp_pkt *fc_fcp_pkt_alloc(struct fc_lport *lp) sp->lp = lp; atomic_set(&sp->ref_cnt, 1); init_timer(&sp->timer); + INIT_LIST_HEAD(&sp->list); } return sp; } @@ -273,12 +279,10 @@ static inline void fc_fcp_unlock_pkt(struct fc_fcp_pkt *fsp) fc_fcp_pkt_release(fsp); } -static void fc_fcp_timer_set(struct fc_fcp_pkt *fsp, u32 delay) +static void fc_fcp_timer_set(struct fc_fcp_pkt *fsp, unsigned long delay) { - if (timer_pending(&fsp->timer)) - del_timer_sync(&fsp->timer); - fsp->timer.expires = jiffies + delay; - add_timer(&fsp->timer); + if (!(fsp->state & FC_SRB_COMPL)) + mod_timer(&fsp->timer, jiffies + delay); } /* @@ -825,64 +829,56 @@ static void fc_fcp_complete(struct fc_fcp_pkt *fsp) } /** - * fc_fcp_for_each_cmd - run fn on each active command + * fc_fcp_cleanup_each_cmd - run fn on each active command * @lp: logical port * @id: target id * @lun: lun * @fn: actor function * * If lun or id is -1, they are ignored. + * + * @fn must not call fs_io_compl on the fsp. */ -static void fc_fcp_for_each_cmd(struct fc_lport *lp, unsigned int id, - unsigned int lun, - void (*fn)(struct fc_fcp_pkt *)) +static void fc_fcp_cleanup_each_cmd(struct fc_lport *lp, unsigned int id, + unsigned int lun, + void (*fn)(struct fc_fcp_pkt *)) { + struct fc_fcp_internal *si = fc_get_scsi_internal(lp); struct fc_fcp_pkt *fsp; struct scsi_cmnd *sc_cmd; unsigned long flags; - int i; - for (i = 0; i < lp->host->hostt->can_queue; i++) { - spin_lock_irqsave(lp->host->host_lock, flags); - sc_cmd = scsi_host_find_tag(lp->host, i); - if (!sc_cmd) { - spin_unlock_irqrestore(lp->host->host_lock, flags); - continue; - } + spin_lock_irqsave(lp->host->host_lock, flags); + while (1) { + list_for_each_entry(fsp, &si->scsi_pkt_queue, list) { + sc_cmd = fsp->cmd; + if (id != -1 && scmd_id(sc_cmd) != id) + continue; - if (id != -1 && scmd_id(sc_cmd) != id) { - spin_unlock_irqrestore(lp->host->host_lock, flags); - continue; - } + if (lun != -1 && sc_cmd->device->lun != lun) + continue; - if (lun != -1 && sc_cmd->device->lun != lun) { + fc_fcp_pkt_hold(fsp); spin_unlock_irqrestore(lp->host->host_lock, flags); - continue; - } - fsp = CMD_SP(sc_cmd); - if (!fsp) { - spin_unlock_irqrestore(lp->host->host_lock, flags); - continue; - } - /* - * grab a ref in case it gets completed when we drop - * the host lock - */ - fc_fcp_pkt_hold(fsp); - spin_unlock_irqrestore(lp->host->host_lock, flags); + if (!fc_fcp_lock_pkt(fsp)) { + fn(fsp); + fc_io_compl(fsp); + fc_fcp_unlock_pkt(fsp); + } - if (fc_fcp_lock_pkt(fsp)) { - /* race - just completed when we drop the host lock */ fc_fcp_pkt_release(fsp); - continue; + spin_lock_irqsave(lp->host->host_lock, flags); + /* + * while we dropped the lock multiple pkts could + * have been released, so we have to start over. + */ + break; } - fn(fsp); - - fc_fcp_unlock_pkt(fsp); - fc_fcp_pkt_release(fsp); + break; } + spin_unlock_irqrestore(lp->host->host_lock, flags); } static void fc_fcp_cleanup_aborted_io(struct fc_fcp_pkt *fsp) @@ -894,16 +890,14 @@ static void fc_fcp_cleanup_aborted_io(struct fc_fcp_pkt *fsp) lp->tt.exch_done(fsp->seq_ptr); fsp->seq_ptr = NULL; } - - fsp->status_code = FC_ERROR; - fsp->io_status = (SUGGEST_RETRY << 24); - fc_io_compl(fsp); } + fsp->status_code = FC_ERROR; + fsp->io_status = (SUGGEST_RETRY << 24); } static void fc_fcp_abort_io(struct fc_lport *lp) { - fc_fcp_for_each_cmd(lp, -1, -1, fc_fcp_cleanup_aborted_io); + fc_fcp_cleanup_each_cmd(lp, -1, -1, fc_fcp_cleanup_aborted_io); } /** @@ -919,6 +913,7 @@ static void fc_fcp_abort_io(struct fc_lport *lp) */ static int fc_fcp_pkt_send(struct fc_lport *lp, struct fc_fcp_pkt *fsp) { + struct fc_fcp_internal *si = fc_get_scsi_internal(lp); int rc; fsp->cmd->SCp.ptr = (char *)fsp; @@ -928,21 +923,35 @@ static int fc_fcp_pkt_send(struct fc_lport *lp, struct fc_fcp_pkt *fsp) int_to_scsilun(fsp->cmd->device->lun, (struct scsi_lun *)fsp->cdb_cmd.fc_lun); memcpy(fsp->cdb_cmd.fc_cdb, fsp->cmd->cmnd, fsp->cmd->cmd_len); + list_add_tail(&fsp->list, &si->scsi_pkt_queue); spin_unlock_irq(lp->host->host_lock); rc = fc_fcp_send_cmd(fsp); spin_lock_irq(lp->host->host_lock); + if (rc) + list_del(&fsp->list); return rc; } +static void fc_fcp_retry_send_cmd(unsigned long data) +{ + fc_fcp_send_cmd((struct fc_fcp_pkt *)data); +} static int fc_fcp_send_cmd(struct fc_fcp_pkt *fsp) { - struct fc_lport *lp = fsp->lp; + struct fc_lport *lp; struct fc_frame *fp; struct fc_seq *sp; struct fc_rport *rport; struct fc_rport_libfc_priv *rp; + if (fc_fcp_lock_pkt(fsp)) + return -1; + + if (fsp->state & FC_SRB_COMPL) + goto unlock; + + lp = fsp->lp; fp = fc_frame_alloc(lp, sizeof(fsp->cdb_cmd)); if (!fp) goto retry; @@ -964,15 +973,18 @@ static int fc_fcp_send_cmd(struct fc_fcp_pkt *fsp) } fsp->seq_ptr = sp; fsp->max_payload = rport->maxframe_size; - fsp->timer.data = (unsigned long)fsp; - fsp->timer.function = (void (*)(unsigned long))fc_fcp_timeout; - fc_fcp_timer_set(fsp, (fsp->tgt_flags & FC_RP_FLAGS_REC_SUPPORTED) ? - FC_SCSI_REC_TOV : FC_SCSI_ER_TIMEOUT); + + setup_timer(&fsp->timer, fc_fcp_timeout, (unsigned long)fsp); + fc_fcp_timer_set(fsp, + (fsp->tgt_flags & FC_RP_FLAGS_REC_SUPPORTED) ? + FC_SCSI_REC_TOV : FC_SCSI_ER_TIMEOUT); +unlock: + fc_fcp_unlock_pkt(fsp); return 0; retry: - fsp->timer.data = (unsigned long)fsp; - fsp->timer.function = (void (*)(unsigned long))fc_fcp_send_cmd; + setup_timer(&fsp->timer, fc_fcp_retry_send_cmd, (unsigned long)fsp); fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV); + fc_fcp_unlock_pkt(fsp); return 0; } @@ -1052,8 +1064,9 @@ static int fc_fcp_pkt_abort(struct fc_lport *lp, struct fc_fcp_pkt *fsp) /* * Retry LUN reset after resource allocation failed. */ -static void fc_lun_reset_send(struct fc_fcp_pkt *fsp) +static void fc_lun_reset_send(unsigned long data) { + struct fc_fcp_pkt *fsp = (struct fc_fcp_pkt *)data; const size_t len = sizeof(fsp->cdb_cmd); struct fc_lport *lp = fsp->lp; struct fc_frame *fp; @@ -1061,6 +1074,10 @@ static void fc_lun_reset_send(struct fc_fcp_pkt *fsp) struct fc_rport *rport; struct fc_rport_libfc_priv *rp; + spin_lock(&fsp->scsi_pkt_lock); + if (fsp->state & FC_SRB_COMPL) + goto unlock; + fp = fc_frame_alloc(lp, len); if (!fp) goto retry; @@ -1078,15 +1095,17 @@ static void fc_lun_reset_send(struct fc_fcp_pkt *fsp) if (sp) { fsp->seq_ptr = sp; - return; + goto unlock; } /* * Exchange or frame allocation failed. Set timer and retry. */ fc_frame_free(fp); retry: - fsp->timer.function = (void (*)(unsigned long))fc_lun_reset_send; + 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); } static void fc_fcp_cleanup_lun_reset(struct fc_fcp_pkt *fsp) @@ -1094,12 +1113,10 @@ static void fc_fcp_cleanup_lun_reset(struct fc_fcp_pkt *fsp) struct fc_lport *lp = fsp->lp; fsp->status_code = FC_CMD_ABORTED; - fsp->cdb_status = -1; if (fsp->seq_ptr) { lp->tt.exch_done(fsp->seq_ptr); fsp->seq_ptr = NULL; } - fc_io_compl(fsp); } /* @@ -1118,13 +1135,20 @@ static int fc_lun_reset(struct fc_lport *lp, struct fc_fcp_pkt *fsp, fsp->wait_for_comp = 1; init_completion(&fsp->tm_done); - fc_lun_reset_send(fsp); + fc_lun_reset_send((unsigned long)fsp); /* * wait for completion of reset * after that make sure all commands are terminated */ rc = wait_for_completion_timeout(&fsp->tm_done, FC_SCSI_TM_TOV); + + spin_lock(&fsp->scsi_pkt_lock); + fsp->state |= FC_SRB_COMPL; + spin_unlock(&fsp->scsi_pkt_lock); + + del_timer_sync(&fsp->timer); + spin_lock(&fsp->scsi_pkt_lock); if (fsp->seq_ptr) { /* TODO: @@ -1149,7 +1173,7 @@ static int fc_lun_reset(struct fc_lport *lp, struct fc_fcp_pkt *fsp, return FAILED; FC_DBG("lun reset to lun %u completed\n", lun); - fc_fcp_for_each_cmd(lp, id, lun, fc_fcp_cleanup_lun_reset); + fc_fcp_cleanup_each_cmd(lp, id, lun, fc_fcp_cleanup_lun_reset); return SUCCESS; } @@ -1167,7 +1191,8 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) * TODO: If this happens we could be freeing the fsp right now and * would oops. Next patches will fix this race. */ - if (!fsp->seq_ptr || !fsp->wait_for_comp) { + if ((fsp->state & FC_SRB_COMPL) || !fsp->seq_ptr || + !fsp->wait_for_comp) { spin_unlock(&fsp->scsi_pkt_lock); return; } @@ -1191,12 +1216,11 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) static void fc_fcp_cleanup_io(struct fc_fcp_pkt *fsp) { fsp->status_code = FC_HRD_ERROR; - fc_fcp_complete(fsp); } static void fc_fcp_cleanup(struct fc_lport *lp) { - fc_fcp_for_each_cmd(lp, -1, -1, fc_fcp_cleanup_io); + fc_fcp_cleanup_each_cmd(lp, -1, -1, fc_fcp_cleanup_io); } /* @@ -1217,18 +1241,24 @@ static void fc_fcp_cleanup(struct fc_lport *lp) * we see if data was received recently. If it has been, we continue waiting, * otherwise, if it is a simple read or write, we abort the command. */ -static void fc_fcp_timeout(struct fc_fcp_pkt *fsp) +static void fc_fcp_timeout(unsigned long data) { + struct fc_fcp_pkt *fsp = (struct fc_fcp_pkt *)data; struct fc_rport *rport = fsp->rport; struct fc_rport_libfc_priv *rp = rport->dd_data; u8 cdb_op; if (fc_fcp_lock_pkt(fsp)) return; + + if (fsp->state & FC_SRB_COMPL) + goto unlock; + cdb_op = fsp->cdb_cmd.fc_cdb[0]; if (rp->flags & FC_RP_FLAGS_REC_SUPPORTED) fc_fcp_rec(fsp); + /* TODO: change this to time_before/after */ else if (jiffies - fsp->last_pkt_time < FC_SCSI_ER_TIMEOUT / 2) fc_fcp_timer_set(fsp, FC_SCSI_ER_TIMEOUT); else if (fsp->state & FC_SRB_RCV_STATUS) @@ -1236,6 +1266,7 @@ static void fc_fcp_timeout(struct fc_fcp_pkt *fsp) else if (cdb_op == READ_10 || cdb_op == READ_6 || cdb_op == WRITE_10 || cdb_op == WRITE_6) fc_timeout_error(fsp); +unlock: fc_fcp_unlock_pkt(fsp); } @@ -1653,8 +1684,6 @@ out: * * this is the i/o strategy routine, called by the scsi layer * this routine is called with holding the host_lock. - * - * When using this function, fc_slave_alloc must be called. */ int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *)) { @@ -1752,7 +1781,7 @@ out: EXPORT_SYMBOL(fc_queuecommand); /** - * fc_io_compl - Handle responses for completed commands + * fc_io_compl - Handle responses for completed commands * @sp: scsi packet * * Translates a error to a Linux SCSI error. @@ -1765,15 +1794,7 @@ static void fc_io_compl(struct fc_fcp_pkt *sp) struct fc_lport *lp; unsigned long flags; - lp = sp->lp; - if (lp == NULL) { - FC_DBG("error could not find libfc hba softc\n"); - return; - } - - if (!sp->cmd) - return; - + sp->state |= FC_SRB_COMPL; if (timer_pending(&sp->timer)) { fc_fcp_unlock_pkt(sp); del_timer_sync(&sp->timer); @@ -1782,10 +1803,16 @@ static void fc_io_compl(struct fc_fcp_pkt *sp) return; } + lp = sp->lp; + spin_lock_irqsave(lp->host->host_lock, flags); + if (!sp->cmd) { + spin_unlock_irqrestore(lp->host->host_lock, flags); + return; + } + sc_cmd = sp->cmd; sp->cmd = NULL; - spin_lock_irqsave(lp->host->host_lock, flags); if (!sc_cmd->SCp.ptr) { spin_unlock_irqrestore(lp->host->host_lock, flags); return; @@ -1868,6 +1895,7 @@ static void fc_io_compl(struct fc_fcp_pkt *sp) break; } + list_del(&sp->list); sc_cmd->SCp.ptr = NULL; sc_cmd->scsi_done(sc_cmd); spin_unlock_irqrestore(lp->host->host_lock, flags); @@ -2020,16 +2048,13 @@ int fc_slave_alloc(struct scsi_device *sdev) if (!rport || fc_remote_port_chkready(rport)) return -ENXIO; - if (sdev->host->hostt->cmd_per_lun) - queue_depth = sdev->host->hostt->cmd_per_lun; - else - queue_depth = FC_FCP_DFLT_QUEUE_DEPTH; - /* - * We set tagged_supported, because we are going to use - * the host wide tag map. - */ - sdev->tagged_supported = 1; - scsi_activate_tcq(sdev, queue_depth); + if (sdev->tagged_supported) { + if (sdev->host->hostt->cmd_per_lun) + queue_depth = sdev->host->hostt->cmd_per_lun; + else + queue_depth = FC_FCP_DFLT_QUEUE_DEPTH; + scsi_activate_tcq(sdev, queue_depth); + } return 0; } EXPORT_SYMBOL(fc_slave_alloc); @@ -2060,6 +2085,9 @@ void fc_fcp_destroy(struct fc_lport *lp) { struct fc_fcp_internal *si = fc_get_scsi_internal(lp); + if (!list_empty(&si->scsi_pkt_queue)) + printk(KERN_ERR "Leaked scsi packets.\n"); + mempool_destroy(si->scsi_pkt_pool); kfree(si); lp->scsi_priv = NULL; @@ -2077,17 +2105,11 @@ int fc_fcp_init(struct fc_lport *lp) if (!lp->tt.scsi_abort_io) lp->tt.scsi_abort_io = fc_fcp_abort_io; - /* - * tag map will be released by scsi_host's release function. - */ - rc = scsi_init_shared_tag_map(lp->host, lp->host->hostt->can_queue); - if (rc) - return rc; - si = kzalloc(sizeof(struct fc_fcp_internal), GFP_KERNEL); if (!si) return -ENOMEM; lp->scsi_priv = si; + INIT_LIST_HEAD(&si->scsi_pkt_queue); si->scsi_pkt_pool = mempool_create_slab_pool(2, scsi_pkt_cachep); if (!si->scsi_pkt_pool) { -- 1.5.4.1 _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
