From: mchristi@redhat.com <mchristi@redhat.com> Date: Tue, 14 Apr 2009 16:00:46 -0500 Subject: [scsi] sym53c8xx_2: fix up hotplug support Message-id: 1239742846-12178-1-git-send-email-mchristi@redhat.com O-Subject: [PATCH] RHEL 5.4 sym53c8xx_2: fix up hotplug support Bugzilla: 461006 RH-Acked-by: Tomas Henzl <thenzl@redhat.com> From: Mike Christie <mchristi@redhat.com> This is for BZ 461006. When doing a controlled hotswap (using sysfs or proc to add/ remove the device then physically remove/add the device), the sym driver is using old transfer settings. This patch fixes that and the following related issues: - Fix for the sym53c8xx_2 driver to initialize lun's to_clear flag after a bus reset (a failed clear can trigger a bus reset and it should not be attemped again after that). - Make the sym53c8xx_2 driver slave_alloc/destroy less unsafe.References to the destroyed LCB are cleared from the target structure (instead of leaving a dangling pointer), and when the last LCB for the target is destroyed the reference to the upper layer target data is cleared. The host lock is used to prevent a race with the interrupt handler. Also user commands are prevented for targets with all LCBs destroyed. The patch to fix the negotion logic is in Linus's tree http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=49799fee82b4f78c45b1926be24e45b5cf667083 The patches for the other two issues are not. The upstream maintainer for the driver OKd them all (here is his reply for the other two patches): http://marc.info/?l=linux-scsi&m=123133995824326&w=2 http://marc.info/?l=linux-scsi&m=123127926710036&w=2 But I think some wires got crossed and only one got merged. I resent those patches here http://marc.info/?l=linux-scsi&m=123974209422486&w=2 http://marc.info/?l=linux-scsi&m=123974209422490&w=2 http://marc.info/?l=linux-scsi&m=123974209522495&w=2 to make sure the scsi maintainer can easily pick them up. Patrick Ernzer has tested the patch: https://bugzilla.redhat.com/show_bug.cgi?id=461006 diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c index cba2060..edd25d6 100644 --- a/drivers/scsi/sym53c8xx_2/sym_glue.c +++ b/drivers/scsi/sym53c8xx_2/sym_glue.c @@ -933,11 +933,14 @@ static int sym53c8xx_slave_alloc(struct scsi_device *sdev) struct sym_hcb *np = sym_get_hcb(sdev->host); struct sym_tcb *tp = &np->target[sdev->id]; struct sym_lcb *lp; + unsigned long flags; + int error; if (sdev->id >= SYM_CONF_MAX_TARGET || sdev->lun >= SYM_CONF_MAX_LUN) return -ENXIO; - tp->starget = sdev->sdev_target; + spin_lock_irqsave(np->s.host->host_lock, flags); + /* * Fail the device init if the device is flagged NOSCAN at BOOT in * the NVRAM. This may speed up boot and maintain coherency with @@ -949,26 +952,37 @@ static int sym53c8xx_slave_alloc(struct scsi_device *sdev) if (tp->usrflags & SYM_SCAN_BOOT_DISABLED) { tp->usrflags &= ~SYM_SCAN_BOOT_DISABLED; - starget_printk(KERN_INFO, tp->starget, + starget_printk(KERN_INFO, sdev->sdev_target, "Scan at boot disabled in NVRAM\n"); - return -ENXIO; + error = -ENXIO; + goto out; } if (tp->usrflags & SYM_SCAN_LUNS_DISABLED) { - if (sdev->lun != 0) - return -ENXIO; - starget_printk(KERN_INFO, tp->starget, + if (sdev->lun != 0) { + error = -ENXIO; + goto out; + } + starget_printk(KERN_INFO, sdev->sdev_target, "Multiple LUNs disabled in NVRAM\n"); } lp = sym_alloc_lcb(np, sdev->id, sdev->lun); - if (!lp) - return -ENOMEM; + if (!lp) { + error = -ENOMEM; + goto out; + } + if (tp->nlcb == 1) + tp->starget = sdev->sdev_target; spi_min_period(tp->starget) = tp->usr_period; spi_max_width(tp->starget) = tp->usr_width; - return 0; + error = 0; +out: + spin_unlock_irqrestore(np->s.host->host_lock, flags); + + return error; } /* @@ -1020,12 +1034,34 @@ static int sym53c8xx_slave_configure(struct scsi_device *sdev) static void sym53c8xx_slave_destroy(struct scsi_device *sdev) { struct sym_hcb *np = sym_get_hcb(sdev->host); - struct sym_lcb *lp = sym_lp(&np->target[sdev->id], sdev->lun); + struct sym_tcb *tp = &np->target[sdev->id]; + struct sym_lcb *lp = sym_lp(tp, sdev->lun); + unsigned long flags; + + spin_lock_irqsave(np->s.host->host_lock, flags); + + if (lp->busy_itlq || lp->busy_itl) { + /* + * This really shouldn't happen, but we can't return an error + * so let's try to stop all on-going I/O. + */ + starget_printk(KERN_WARNING, tp->starget, + "Removing busy LCB (%d)\n", sdev->lun); + sym_reset_scsi_bus(np, 1); + } - if (lp->itlq_tbl) - sym_mfree_dma(lp->itlq_tbl, SYM_CONF_MAX_TASK * 4, "ITLQ_TBL"); - kfree(lp->cb_tags); - sym_mfree_dma(lp, sizeof(*lp), "LCB"); + if (sym_free_lcb(np, sdev->id, sdev->lun) == 0) { + /* + * It was the last unit for this target. + */ + tp->head.sval = 0; + tp->head.wval = np->rv_scntl3; + tp->head.uval = 0; + tp->tgoal.check_nego = 1; + tp->starget = NULL; + } + + spin_unlock_irqrestore(np->s.host->host_lock, flags); } /* @@ -1091,6 +1127,8 @@ static void sym_exec_user_command (struct sym_hcb *np, struct sym_usrcmd *uc) if (!((uc->target >> t) & 1)) continue; tp = &np->target[t]; + if (!tp->nlcb) + continue; switch (uc->cmd) { diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c index 6c64a33..8e6e21a 100644 --- a/drivers/scsi/sym53c8xx_2/sym_hipd.c +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c @@ -1436,13 +1436,12 @@ static int sym_prepare_nego(struct sym_hcb *np, struct sym_ccb *cp, u_char *msgp * Many devices implement PPR in a buggy way, so only use it if we * really want to. */ - if (goal->offset && - (goal->iu || goal->dt || goal->qas || (goal->period < 0xa))) { + if (goal->renego == NS_PPR || (goal->offset && + (goal->iu || goal->dt || goal->qas || (goal->period < 0xa)))) { nego = NS_PPR; - } else if (spi_width(starget) != goal->width) { + } else if (goal->renego == NS_WIDE || goal->width) { nego = NS_WIDE; - } else if (spi_period(starget) != goal->period || - spi_offset(starget) != goal->offset) { + } else if (goal->renego == NS_SYNC || goal->offset) { nego = NS_SYNC; } else { goal->check_nego = 0; @@ -1896,6 +1895,15 @@ void sym_start_up (struct sym_hcb *np, int reason) tp->head.sval = 0; tp->head.wval = np->rv_scntl3; tp->head.uval = 0; + if (tp->lun0p) + tp->lun0p->to_clear = 0; + if (tp->lunmp) { + int ln; + + for (ln = 1; ln < SYM_CONF_MAX_LUN; ln++) + if (tp->lunmp[ln]) + tp->lunmp[ln]->to_clear = 0; + } } /* @@ -2048,11 +2056,13 @@ static void sym_setwide(struct sym_hcb *np, int target, u_char wide) struct sym_tcb *tp = &np->target[target]; struct scsi_target *starget = tp->starget; - if (spi_width(starget) == wide) - return; - sym_settrans(np, target, 0, 0, 0, wide, 0, 0); + if (wide) + tp->tgoal.renego = NS_WIDE; + else + tp->tgoal.renego = 0; + tp->tgoal.check_nego = 0; tp->tgoal.width = wide; spi_offset(starget) = 0; spi_period(starget) = 0; @@ -2079,6 +2089,12 @@ sym_setsync(struct sym_hcb *np, int target, sym_settrans(np, target, 0, ofs, per, wide, div, fak); + if (wide) + tp->tgoal.renego = NS_WIDE; + else if (ofs) + tp->tgoal.renego = NS_SYNC; + else + tp->tgoal.renego = 0; spi_period(starget) = per; spi_offset(starget) = ofs; spi_iu(starget) = spi_dt(starget) = spi_qas(starget) = 0; @@ -2105,6 +2121,10 @@ sym_setpprot(struct sym_hcb *np, int target, u_char opts, u_char ofs, sym_settrans(np, target, opts, ofs, per, wide, div, fak); + if (wide || ofs) + tp->tgoal.renego = NS_PPR; + else + tp->tgoal.renego = 0; spi_width(starget) = tp->tgoal.width = wide; spi_period(starget) = tp->tgoal.period = per; spi_offset(starget) = tp->tgoal.offset = ofs; @@ -3509,6 +3529,7 @@ static void sym_sir_task_recovery(struct sym_hcb *np, int num) spi_dt(starget) = 0; spi_qas(starget) = 0; tp->tgoal.check_nego = 1; + tp->tgoal.renego = 0; } /* @@ -4947,7 +4968,7 @@ struct sym_lcb *sym_alloc_lcb (struct sym_hcb *np, u_char tn, u_char ln) */ if (ln && !tp->lunmp) { tp->lunmp = kcalloc(SYM_CONF_MAX_LUN, sizeof(struct sym_lcb *), - GFP_KERNEL); + GFP_ATOMIC); if (!tp->lunmp) goto fail; } @@ -4967,6 +4988,7 @@ struct sym_lcb *sym_alloc_lcb (struct sym_hcb *np, u_char tn, u_char ln) tp->lun0p = lp; tp->head.lun0_sa = cpu_to_scr(vtobus(lp)); } + tp->nlcb++; /* * Let the itl task point to error handling. @@ -5044,6 +5066,43 @@ fail: } /* + * Lun control block deallocation. Returns the number of valid remaing LCBs + * for the target. + */ +int sym_free_lcb(struct sym_hcb *np, u_char tn, u_char ln) +{ + struct sym_tcb *tp = &np->target[tn]; + struct sym_lcb *lp = sym_lp(tp, ln); + + tp->nlcb--; + + if (ln) { + if (!tp->nlcb) { + kfree(tp->lunmp); + sym_mfree_dma(tp->luntbl, 256, "LUNTBL"); + tp->lunmp = NULL; + tp->luntbl = NULL; + tp->head.luntbl_sa = cpu_to_scr(vtobus(np->badluntbl)); + } else { + tp->luntbl[ln] = cpu_to_scr(vtobus(&np->badlun_sa)); + tp->lunmp[ln] = NULL; + } + } else { + tp->lun0p = NULL; + tp->head.lun0_sa = cpu_to_scr(vtobus(&np->badlun_sa)); + } + + if (lp->itlq_tbl) { + sym_mfree_dma(lp->itlq_tbl, SYM_CONF_MAX_TASK*4, "ITLQ_TBL"); + kfree(lp->cb_tags); + } + + sym_mfree_dma(lp, sizeof(*lp), "LCB"); + + return tp->nlcb; +} + +/* * Queue a SCSI IO to the controller. */ int sym_queue_scsiio(struct sym_hcb *np, struct scsi_cmnd *cmd, struct sym_ccb *cp) @@ -5129,9 +5188,14 @@ int sym_queue_scsiio(struct sym_hcb *np, struct scsi_cmnd *cmd, struct sym_ccb * /* * Build a negotiation message if needed. * (nego_status is filled by sym_prepare_nego()) + * + * Always negotiate on INQUIRY and REQUEST SENSE. + * */ cp->nego_status = 0; - if (tp->tgoal.check_nego && !tp->nego_cp && lp) { + if ((tp->tgoal.check_nego || + cmd->cmnd[0] == INQUIRY || cmd->cmnd[0] == REQUEST_SENSE) && + !tp->nego_cp && lp) { msglen += sym_prepare_nego(np, cp, msgptr + msglen); } diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h index 79ab6a1..c1e2c84 100644 --- a/drivers/scsi/sym53c8xx_2/sym_hipd.h +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h @@ -354,6 +354,7 @@ struct sym_trans { unsigned int dt:1; unsigned int qas:1; unsigned int check_nego:1; + unsigned int renego:2; }; /* @@ -400,6 +401,7 @@ struct sym_tcb { * An array of bus addresses is used on reselection. */ u32 *luntbl; /* LCBs bus address table */ + int nlcb; /* Number of valid LCBs (including LUN #0) */ /* * LUN table used by the C code. @@ -1058,6 +1060,7 @@ int sym_clear_tasks(struct sym_hcb *np, int cam_status, int target, int lun, int struct sym_ccb *sym_get_ccb(struct sym_hcb *np, struct scsi_cmnd *cmd, u_char tag_order); void sym_free_ccb(struct sym_hcb *np, struct sym_ccb *cp); struct sym_lcb *sym_alloc_lcb(struct sym_hcb *np, u_char tn, u_char ln); +int sym_free_lcb(struct sym_hcb *np, u_char tn, u_char ln); int sym_queue_scsiio(struct sym_hcb *np, struct scsi_cmnd *csio, struct sym_ccb *cp); int sym_abort_scsiio(struct sym_hcb *np, struct scsi_cmnd *ccb, int timed_out); int sym_reset_scsi_target(struct sym_hcb *np, int target);