[Bug 76681] Adaptec 7805H SAS HBA (pm80xx) cannot survive ACPI S3 or ACPI S4
https://bugzilla.kernel.org/show_bug.cgi?id=76681 Jack Wang changed: What|Removed |Added CC||xjtu...@gmail.com --- Comment #1 from Jack Wang --- Sorry, I don't have hardware to test, could you try patch below: diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index f7c1896..083a030 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -916,6 +916,7 @@ static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state) int i; u32 device_state; pm8001_ha = sha->lldd_ha; + sas_suspend_ha(sha); flush_workqueue(pm8001_wq); scsi_block_requests(pm8001_ha->shost); if (!pdev->pm_cap) { @@ -980,7 +981,7 @@ static int pm8001_pci_resume(struct pci_dev *pdev) rc = pci_go_44(pdev); if (rc) goto err_out_disable; - + sas_prep_resume_ha(sha); /* chip soft rst only for spc */ if (pm8001_ha->chip_id == chip_8001) { PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha); @@ -1009,6 +1010,9 @@ static int pm8001_pci_resume(struct pci_dev *pdev) PM8001_CHIP_DISP->interrupt_enable(pm8001_ha, i); } scsi_unblock_requests(pm8001_ha->shost); + pm8001_scan_start(pm8001_ha->shost); + msleep(100); + sas_resume_ha(sha); return 0; err_out_disable: Jack -- You are receiving this mail because: You are the assignee for the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
Il 22/05/2014 04:26, Nicholas A. Bellinger ha scritto: From: Nicholas Bellinger This patch updates virtscsi_probe() to setup necessary Scsi_Host level protection resources. (currently hardcoded to 1) It changes virtscsi_add_cmd() to attach outgoing / incoming protection SGLs preceeding the data payload, and is using the new virtio_scsi_cmd_req_pi->d[oi],pi_niv field to signal Wrong field name in the commit message... to signal to vhost/scsi how many prot_sgs to expect. s/prot_sgs to expect/bytes to expect for protection data/ Apart from this, which can be fixed in the pull request, Acked-by: Paolo Bonzini for getting this in via the target tree. Paolo v4 changes: - Convert virtio_scsi_init_hdr_pi to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity->tuple_size to calculate pi bytes (nab) v3 changes: - Use VIRTIO_SCSI_F_T10_PI to determine PI or non PI header (Paolo) v2 changes: - Make protection buffer come before data buffer (Paolo) - Enable virtio_scsi_cmd_req_pi usage (Paolo) Cc: Paolo Bonzini Cc: Michael S. Tsirkin Cc: Martin K. Petersen Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Sagi Grimberg Cc: H. Peter Anvin Signed-off-by: Nicholas Bellinger --- drivers/scsi/virtio_scsi.c | 86 ++-- 1 file changed, 68 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 16bfd50..cc634b0 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -37,6 +37,7 @@ struct virtio_scsi_cmd { struct completion *comp; union { struct virtio_scsi_cmd_req cmd; + struct virtio_scsi_cmd_req_picmd_pi; struct virtio_scsi_ctrl_tmf_req tmf; struct virtio_scsi_ctrl_an_req an; } req; @@ -440,7 +441,7 @@ static int virtscsi_add_cmd(struct virtqueue *vq, size_t req_size, size_t resp_size, gfp_t gfp) { struct scsi_cmnd *sc = cmd->sc; - struct scatterlist *sgs[4], req, resp; + struct scatterlist *sgs[6], req, resp; struct sg_table *out, *in; unsigned out_num = 0, in_num = 0; @@ -458,16 +459,24 @@ static int virtscsi_add_cmd(struct virtqueue *vq, sgs[out_num++] = &req; /* Data-out buffer. */ - if (out) + if (out) { + /* Place WRITE protection SGLs before Data OUT payload */ + if (scsi_prot_sg_count(sc)) + sgs[out_num++] = scsi_prot_sglist(sc); sgs[out_num++] = out->sgl; + } /* Response header. */ sg_init_one(&resp, &cmd->resp, resp_size); sgs[out_num + in_num++] = &resp; /* Data-in buffer */ - if (in) + if (in) { + /* Place READ protection SGLs before Data IN payload */ + if (scsi_prot_sg_count(sc)) + sgs[out_num + in_num++] = scsi_prot_sglist(sc); sgs[out_num + in_num++] = in->sgl; + } return virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, gfp); } @@ -492,12 +501,44 @@ static int virtscsi_kick_cmd(struct virtio_scsi_vq *vq, return err; } +static void virtio_scsi_init_hdr(struct virtio_scsi_cmd_req *cmd, +struct scsi_cmnd *sc) +{ + cmd->lun[0] = 1; + cmd->lun[1] = sc->device->id; + cmd->lun[2] = (sc->device->lun >> 8) | 0x40; + cmd->lun[3] = sc->device->lun & 0xff; + cmd->tag = (unsigned long)sc; + cmd->task_attr = VIRTIO_SCSI_S_SIMPLE; + cmd->prio = 0; + cmd->crn = 0; +} + +static void virtio_scsi_init_hdr_pi(struct virtio_scsi_cmd_req_pi *cmd_pi, + struct scsi_cmnd *sc) +{ + struct request *rq = sc->request; + struct blk_integrity *bi; + + virtio_scsi_init_hdr((struct virtio_scsi_cmd_req *)cmd_pi, sc); + + if (!rq || !scsi_prot_sg_count(sc)) + return; + + bi = blk_get_integrity(rq->rq_disk); + + if (sc->sc_data_direction == DMA_TO_DEVICE) + cmd_pi->pi_bytesout = blk_rq_sectors(rq) * bi->tuple_size; + else if (sc->sc_data_direction == DMA_FROM_DEVICE) + cmd_pi->pi_bytesin = blk_rq_sectors(rq) * bi->tuple_size; +} + static int virtscsi_queuecommand(struct virtio_scsi *vscsi, struct virtio_scsi_vq *req_vq, struct scsi_cmnd *sc) { struct virtio_scsi_cmd *cmd; - int ret; + int ret, req_size; struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev); BUG_ON(scsi_sg_count(sc) > shost->sg_tablesize); @@ -515,22 +556,20 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi, memset(cmd, 0, sizeof(*cmd)); cmd->sc = sc; - cmd->req.cmd = (struct virtio_scsi_cmd_req){ - .lun[0] = 1, - .lun[1] = sc->device->id, -
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
Il 22/05/2014 04:26, Nicholas A. Bellinger ha scritto: From: Nicholas Bellinger Hi MST, MKP, Paolo & Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + ->tuple_size to calculate the total bytes for outgoing cmd_pi->pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity->tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV -> SGL memory mapping logic vhost/scsi: Enable T10 PI IOV -> SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) Looks good, thanks! Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] scsi: Output error messages using structured printk in single line
On Wed, May 21, 2014 at 08:30:58AM +0200, Hannes Reinecke wrote: > While this works reasonably well for most things, printing out > decoded sense with just one line (and not end up in massive switch() > statements) is near impossible. > > Plus you'll end up having to use a static buffer at one point, which > again increases the stack size. > > The alternative approach as discussed at LSF is to move scsi_logging > over to tracing. There is already some coding for scsi tracing, but > in most cases it just duplicates existing logging statements. > So if we were to replace the entire scsi_logging infrastructure > with scsi tracing most of the issues (like chopped-up CDBs) would > be gone. > Plus we would have a far better control about _what_ is being printed. > > And yes, I do have some patches for that :-) I think any detailed logging of sense codes, cdbs and co should move to tracing. In fact generally any non-defauly logging probably is better off in the tracing subsystems, but there might be a few exceptions. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] scsi: Remove CONFIG_SCSI_MULTI_LUN
On Tue, May 20, 2014 at 01:03:07PM +0200, Hannes Reinecke wrote: > Obsolete; either use 'max_lun' if the host supports only a > limited number of LUNs or BLIST_NOLUN if the target has > problems addressing more than one LUN. > > Signed-off-by: Hannes Reinecke Looks good, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] scsi_scan: Restrict sequential scan to 256 LUNs
On Tue, May 20, 2014 at 01:03:08PM +0200, Hannes Reinecke wrote: > Sequential scan for more than 256 LUNs is very fragile as > LUNs might not be numbered sequentially after that point. > > SAM revisions later than SCSI-3 impose a structure on > LUNs larger than 256, making LUN numbers between 256 > and 16384 illegal. > SCSI-3, however allows for plain 64-bit numbers with > no internal structure. > > So restrict sequential LUN scan to 256 LUNs and add a > new blacklist flag 'BLIST_SCSI3LUN' to scan up to > max_lun devices. Looks good, Reviewed-by: Christoph Hellwig Do you know any common devices reporting SCSI-2? I've only really seen SCSI-2 and never SBC levels in practical use. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] qla2xxx: Restrict max_lun to 16-bit for older HBAs
On Tue, May 20, 2014 at 01:03:09PM +0200, Hannes Reinecke wrote: > Older HBAs are only capable of supporting 16-bit LUNs, > so we need to make sure to adjust max_lun accordingly. > > Signed-off-by: Hannes Reinecke > Acked-by: Chad Dupuis Looks good, Reviewed-by: Christoph Hellwig This is probably something we should put in now instead of waiting for the rest of the series. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] scsi: use 64-bit LUNs
On Tue, May 20, 2014 at 01:03:10PM +0200, Hannes Reinecke wrote: > The SCSI standard defines 64-bit values for LUNs, and large arrays > employing large or hierarchical LUN numbers become more and more > common. > > So update the linux SCSI stack to use 64-bit LUN numbers. > > Signed-off-by: Hannes Reinecke Looks good, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] scsi: use 64-bit value for 'max_luns'
On Tue, May 20, 2014 at 01:03:11PM +0200, Hannes Reinecke wrote: > Now that we're using 64-bit LUNs internally we need to increase > the size of max_luns to 64 bits, too. Looks good, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
> -Original Message- > From: Andy Whitcroft [mailto:a...@canonical.com] > Sent: Wednesday, May 21, 2014 7:25 AM > To: Ian Abbott > Cc: linux-scsi@vger.kernel.org; de...@linuxdriverproject.org; KY Srinivasan; > Haiyang Zhang; James E.J. Bottomley; Tim Gardner > Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu > > On 16 May 2014 16:39, Ian Abbott wrote: > > These changes to the Microsoft Hyper-V storage driver in Ubuntu > > Saucy's > > 3.13 kernel look useful for the mainline kernel, especially as they > > enable 'TRIM' support. > > > > Andy Whitcroft (2): > > scsi: hyper-v storvsc switch up to SPC-3 > > scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- > > elide them > > > > drivers/scsi/storvsc_drv.c | 10 ++ > > 1 file changed, 10 insertions(+) > > > The back story here is a little complex. The main issue is that the Hyper-V > drives claim to be SPC-2, and yet implement the SPC-3 extensions for TRIM. > We did attempt to upstream quirks to allow these features to be negotiated > specifically for the Hyper-V virtual drives (minimum regression potential) but > these were NAKd, and it was suggested that just overriding the Hyper-V > drives to SPC-3 unconditionally was more appropriate. The first of the > patches here does does this. This has been sitting in our tree for some time > as it was not clear that this would be entirely safe, though the SPC-3 bits > are > in theory at least mostly detected. That said this change has been in Ubuntu > for a full cycle now and does not seem to have caused any issues. If KY is > happy we should likely submit it formally. The second fix I believed was > already being submitted to mainline. > > KY? The Windows guys are not currently comfortable claiming conformance to SPC-3, as they have not done the necessary testing. This will change hopefully soon. K. Y > > -apw N�r��yb�X��ǧv�^�){.n�+{���"�{ay�ʇڙ�,j��f���h���z��w��� ���j:+v���w�j�mzZ+�ݢj"��!�i
Re: [PATCH-v2 1/6] virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits
"Michael S. Tsirkin" writes: > On Thu, May 22, 2014 at 02:26:17AM +, Nicholas A. Bellinger wrote: >> From: Nicholas Bellinger >> >> This patch adds a virtio_scsi_cmd_req_pi header as recommened by >> Paolo that contains do_pi_niov + di_pi_niov elements used for >> signaling when protection information buffers are expected to >> preceed the data buffers. > > Cc rusty. > Rusty could you please Ack merging this through Nicholas' tree > together with the vhost changes? Acked-by: Rusty Russell Thanks, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
On Thu, 2014-05-22 at 10:49 +, KY Srinivasan wrote: > > > > -Original Message- > > From: Andy Whitcroft [mailto:a...@canonical.com] > > Sent: Wednesday, May 21, 2014 7:25 AM > > To: Ian Abbott > > Cc: linux-scsi@vger.kernel.org; de...@linuxdriverproject.org; KY Srinivasan; > > Haiyang Zhang; James E.J. Bottomley; Tim Gardner > > Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu > > > > On 16 May 2014 16:39, Ian Abbott wrote: > > > These changes to the Microsoft Hyper-V storage driver in Ubuntu > > > Saucy's > > > 3.13 kernel look useful for the mainline kernel, especially as they > > > enable 'TRIM' support. > > > > > > Andy Whitcroft (2): > > > scsi: hyper-v storvsc switch up to SPC-3 > > > scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- > > > elide them > > > > > > drivers/scsi/storvsc_drv.c | 10 ++ > > > 1 file changed, 10 insertions(+) > > > > > > The back story here is a little complex. The main issue is that the Hyper-V > > drives claim to be SPC-2, and yet implement the SPC-3 extensions for TRIM. > > We did attempt to upstream quirks to allow these features to be negotiated > > specifically for the Hyper-V virtual drives (minimum regression potential) > > but > > these were NAKd, and it was suggested that just overriding the Hyper-V > > drives to SPC-3 unconditionally was more appropriate. The first of the > > patches here does does this. This has been sitting in our tree for some > > time > > as it was not clear that this would be entirely safe, though the SPC-3 bits > > are > > in theory at least mostly detected. That said this change has been in > > Ubuntu > > for a full cycle now and does not seem to have caused any issues. If KY is > > happy we should likely submit it formally. The second fix I believed was > > already being submitted to mainline. > > > > KY? > > The Windows guys are not currently comfortable claiming conformance to > SPC-3, as they have not done > the necessary testing. This will change hopefully soon. Any bounds on the value of "soon" are we talking weeks or months? I think trim is a feature, which means no huge rush to get this in, but it is nice to respond in a timely fashion to a request from a distribution to enable a useful feature. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()
Il 21/05/2014 15:30, Bart Van Assche ha scritto: +static bool scmd_being_handled_in_other_context(struct scsi_cmnd *scmd) +{ + struct Scsi_Host *shost = scmd->device->host; + struct scsi_cmnd *c; + unsigned long flags; + bool ret = false; + + if (!blk_rq_completed(scmd->request)) + return true; + + spin_lock_irqsave(shost->host_lock, flags); + list_for_each_entry(c, &shost->eh_cmd_q, eh_entry) { + if (c == scmd) { + ret = true; + break; + } + } + spin_unlock_irqrestore(shost->host_lock, flags); + + return ret; +} + /** * scmd_eh_abort_handler - Handle command aborts * @work: command to be aborted. @@ -120,6 +142,8 @@ scmd_eh_abort_handler(struct work_struct *work) struct scsi_device *sdev = scmd->device; int rtn; + WARN_ON_ONCE(scmd_being_handled_in_other_context(scmd)); What about a simpler, though less accuracte WARN_ON(!blk_rq_completed(scmd->request)); that doesn't need the host_lock? Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2i: Make boot_nic entry visible in the sysfs session objects
On 05/19/2014 06:32 AM, vikas.chaudh...@qlogic.com wrote: > From: Tej Parkash > > Signed-off-by: Tej Parkash > Signed-off-by: Vikas Chaudhary > --- > drivers/scsi/bnx2i/bnx2i_iscsi.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c > b/drivers/scsi/bnx2i/bnx2i_iscsi.c > index 166543f..cabc8c1 100644 > --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c > +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c > @@ -2234,6 +2234,9 @@ static umode_t bnx2i_attr_is_visible(int param_type, > int param) > case ISCSI_PARAM_TGT_RESET_TMO: > case ISCSI_PARAM_IFACE_NAME: > case ISCSI_PARAM_INITIATOR_NAME: > + case ISCSI_PARAM_BOOT_ROOT: > + case ISCSI_PARAM_BOOT_NIC: > + case ISCSI_PARAM_BOOT_TARGET: > return S_IRUGO; > default: > return 0; I think we also wanted this for the chelsio driver, but I need to set it up to test. So for now, looks ok to me. If chelsio needs it, I can send a patch for that when done testing. Reviewed-by: Mike Christie -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()
On 05/22/14 18:22, Paolo Bonzini wrote: > Il 21/05/2014 15:30, Bart Van Assche ha scritto: >> +static bool scmd_being_handled_in_other_context(struct scsi_cmnd *scmd) >> +{ >> +struct Scsi_Host *shost = scmd->device->host; >> +struct scsi_cmnd *c; >> +unsigned long flags; >> +bool ret = false; >> + >> +if (!blk_rq_completed(scmd->request)) >> +return true; >> + >> +spin_lock_irqsave(shost->host_lock, flags); >> +list_for_each_entry(c, &shost->eh_cmd_q, eh_entry) { >> +if (c == scmd) { >> +ret = true; >> +break; >> +} >> +} >> +spin_unlock_irqrestore(shost->host_lock, flags); >> + >> +return ret; >> +} >> + >> /** >> * scmd_eh_abort_handler - Handle command aborts >> * @work:command to be aborted. >> @@ -120,6 +142,8 @@ scmd_eh_abort_handler(struct work_struct *work) >> struct scsi_device *sdev = scmd->device; >> int rtn; >> >> +WARN_ON_ONCE(scmd_being_handled_in_other_context(scmd)); > > What about a simpler, though less accuracte > > WARN_ON(!blk_rq_completed(scmd->request)); > > that doesn't need the host_lock? One reason why I posted this patch as an RFC was to invite feedback. I'm fine with leaving out the loop over the eh_cmd_q list although I do not expect that will make a significant performance difference. None of the functions in which a check was added are in the hot path. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [SCSI] mptfusion: remove check for CONFIG_FUSION_MAX_FC_SGE
A check for CONFIG_FUSION_MAX_FC_SGE was added in v2.6.31. But the related Kconfig symbol was never added to the tree. Remove this check, as it always evaluates to false. Signed-off-by: Paul Bolle --- Compile tested only. drivers/message/fusion/mptbase.h | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h index 76c05bc24cb7..43695ec7ac83 100644 --- a/drivers/message/fusion/mptbase.h +++ b/drivers/message/fusion/mptbase.h @@ -177,17 +177,7 @@ #define MPT_SCSI_SG_DEPTH 40 #endif -#ifdef CONFIG_FUSION_MAX_FC_SGE -#if CONFIG_FUSION_MAX_FC_SGE < 16 -#define MPT_SCSI_FC_SG_DEPTH 16 -#elif CONFIG_FUSION_MAX_FC_SGE > 256 -#define MPT_SCSI_FC_SG_DEPTH 256 -#else -#define MPT_SCSI_FC_SG_DEPTH CONFIG_FUSION_MAX_FC_SGE -#endif -#else #define MPT_SCSI_FC_SG_DEPTH 40 -#endif /* debug print string length used for events and iocstatus */ # define EVENT_DESCR_STR_SZ 100 -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SCSI] mptfusion: remove check for CONFIG_FUSION_MAX_FC_SGE
On Thu, 2014-05-22 at 19:58 +, Address Change LSI wrote: > The person you are trying to email is no longer receiving email @lsi.com. > Please try sending to them at > Avago Technologies using firstname.lastn...@avagotech.com. > > Please Note: Your original message has been forwarded to this recipient’s new > Avago email account. It seems lsi.com addresses in MAINTAINERS should be be updated. Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 1/6] virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits
On Thu, 2014-05-22 at 09:57 +0300, Michael S. Tsirkin wrote: > On Thu, May 22, 2014 at 02:26:17AM +, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger > > > > This patch adds a virtio_scsi_cmd_req_pi header as recommened by > > Paolo that contains do_pi_niov + di_pi_niov elements used for > > signaling when protection information buffers are expected to > > preceed the data buffers. > > Cc rusty. > Rusty could you please Ack merging this through Nicholas' tree > together with the vhost changes? > > > > Also add new VIRTIO_SCSI_F_T10_PI feature bit to be used to signal > > host support. > > > > v4 changes: > > v2? Proably best to drop versioning info from commit log, > move it out to notes (after ---) Dropped > > >- Use pi_bytesout + pi_bytesin instead of niovs (mst + paolo) > > Right, so maybe update the commit log above to match? > It gave me pause. Fixed > > > Cc: Paolo Bonzini > > Cc: Michael S. Tsirkin > > Cc: Martin K. Petersen > > Cc: Christoph Hellwig > > Cc: Hannes Reinecke > > Cc: Sagi Grimberg > > Cc: H. Peter Anvin > > Signed-off-by: Nicholas Bellinger > > --- > > include/linux/virtio_scsi.h | 15 ++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h > > index 4195b97..7344906 100644 > > --- a/include/linux/virtio_scsi.h > > +++ b/include/linux/virtio_scsi.h > > @@ -35,11 +35,23 @@ struct virtio_scsi_cmd_req { > > u8 lun[8]; /* Logical Unit Number */ > > u64 tag;/* Command identifier */ > > u8 task_attr; /* Task attribute */ > > - u8 prio; > > + u8 prio;/* SAM command priority field */ > > u8 crn; > > u8 cdb[VIRTIO_SCSI_CDB_SIZE]; > > } __packed; > > > > +/* SCSI command request, followed by protection information */ > > +struct virtio_scsi_cmd_req_pi { > > + u8 lun[8]; /* Logical Unit Number */ > > + u64 tag;/* Command identifier */ > > + u8 task_attr; /* Task attribute */ > > + u8 prio;/* SAM command priority field */ > > + u8 crn; > > + u32 pi_bytesout;/* DataOUT PI Number of bytes */ > > + u32 pi_bytesin; /* DataIN PI Number of bytes */ > > + u8 cdb[VIRTIO_SCSI_CDB_SIZE]; > > +} __packed; > > + > > /* Response, followed by sense data and data-in */ > > struct virtio_scsi_cmd_resp { > > u32 sense_len; /* Sense data length */ > > @@ -97,6 +109,7 @@ struct virtio_scsi_config { > > #define VIRTIO_SCSI_F_INOUT0 > > #define VIRTIO_SCSI_F_HOTPLUG 1 > > #define VIRTIO_SCSI_F_CHANGE 2 > > +#define VIRTIO_SCSI_F_T10_PI 3 > > Could you pad this with spaces and not tabs so it aligns nicely > with stuff above it? Fixed -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Thu, 2014-05-22 at 10:37 +0200, Paolo Bonzini wrote: > Il 22/05/2014 04:26, Nicholas A. Bellinger ha scritto: > > From: Nicholas Bellinger > > > > This patch updates virtscsi_probe() to setup necessary Scsi_Host > > level protection resources. (currently hardcoded to 1) > > > > It changes virtscsi_add_cmd() to attach outgoing / incoming > > protection SGLs preceeding the data payload, and is using the > > new virtio_scsi_cmd_req_pi->d[oi],pi_niv field to signal > > Wrong field name in the commit message... > Fixed > > to signal to vhost/scsi how many prot_sgs to expect. > > s/prot_sgs to expect/bytes to expect for protection data/ > Fixed. > Apart from this, which can be fixed in the pull request, > > Acked-by: Paolo Bonzini > > for getting this in via the target tree. > Thanks Paolo! --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [target:for-next 20/20] drivers/scsi/virtio_scsi.c:531:48: error: dereferencing pointer to incomplete type
Hi Fengguang, On Thu, 2014-05-22 at 11:13 +0800, kbuild test robot wrote: > tree: git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git > for-next > head: 4baaa7d589e24bfe87dfd6c7a1229108be404a28 > commit: 4baaa7d589e24bfe87dfd6c7a1229108be404a28 [20/20] virtio-scsi: Enable > DIF/DIX modes in SCSI host LLD > config: x86_64-randconfig-x000 (attached as .config) > > All error/warnings: > >drivers/scsi/virtio_scsi.c: In function 'virtio_scsi_init_hdr_pi': > >> drivers/scsi/virtio_scsi.c:531:48: error: dereferencing pointer to > >> incomplete type > cmd_pi->pi_bytesout = blk_rq_sectors(rq) * bi->tuple_size; >^ > >> drivers/scsi/virtio_scsi.c:533:47: error: dereferencing pointer to > >> incomplete type > cmd_pi->pi_bytesin = blk_rq_sectors(rq) * bi->tuple_size; > ^ > > vim +531 drivers/scsi/virtio_scsi.c > >525if (!rq || !scsi_prot_sg_count(sc)) >526return; >527 >528bi = blk_get_integrity(rq->rq_disk); >529 >530if (sc->sc_data_direction == DMA_TO_DEVICE) > > 531cmd_pi->pi_bytesout = blk_rq_sectors(rq) * > bi->tuple_size; >532else if (sc->sc_data_direction == DMA_FROM_DEVICE) > > 533cmd_pi->pi_bytesin = blk_rq_sectors(rq) * > bi->tuple_size; >534} >535 >536static int virtscsi_queuecommand(struct virtio_scsi *vscsi, > Squashing the following into the original commit to enable blk-integrity for virtio-scsi to address the randconfig build failure above. Thanks! --nab diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 02832d6..baca589 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -1773,6 +1773,7 @@ config SCSI_BFA_FC config SCSI_VIRTIO tristate "virtio-scsi support" depends on VIRTIO + select BLK_DEV_INTEGRITY help This is the virtual HBA driver for virtio. If the kernel will be used in a virtual machine, say Y or M. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi_transport_sas: move bsg destructor into sas_rphy_remove
The recent change in sysfs, bcdde7e221a8750f9b62b6d0bd31b72ea4ad9309 "sysfs: make __sysfs_remove_dir() recursive" revealed an asymmetric rphy device creation/deletion sequence in scsi_transport_sas: modprobe mpt2sas sas_rphy_add device_add A rphy->dev device_add B sas_device transport class device_add C sas_end_device transport class device_add D bsg class rmmod mpt2sas sas_rphy_delete sas_rphy_remove device_del B device_del C device_del A sysfs_remove_group recursive sysfs dir removal sas_rphy_free device_del D warning where device A is the parent of B, C, and D. When sas_rphy_free tries to unregister the bsg request queue (device D above), the ensuing sysfs cleanup discovers that its sysfs group has already been removed and emits a warning, "sysfs group... not found for kobject 'end_device-X:0'". Since bsg creation is a side effect of sas_rphy_add, move its complementary removal call into sas_rphy_remove. This imposes the following tear-down order for the devices above: D, B, C, A. Note the sas_device and sas_end_device transport class devices (B and C above) are created and destroyed both via the list match traversal in attribute_container_device_trigger, so the order in which they are handled is fixed. This is fine as long as they are deleted before their parent device. Signed-off-by: Joe Lawrence Cc: "James E.J. Bottomley" Cc: Christoph Hellwig Acked-by: Dan Williams --- drivers/scsi/scsi_transport_sas.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c index 1b68142..c341f85 100644 --- a/drivers/scsi/scsi_transport_sas.c +++ b/drivers/scsi/scsi_transport_sas.c @@ -1621,8 +1621,6 @@ void sas_rphy_free(struct sas_rphy *rphy) list_del(&rphy->list); mutex_unlock(&sas_host->lock); - sas_bsg_remove(shost, rphy); - transport_destroy_device(dev); put_device(dev); @@ -1681,6 +1679,7 @@ sas_rphy_remove(struct sas_rphy *rphy) } sas_rphy_unlink(rphy); + sas_bsg_remove(NULL, rphy); transport_remove_device(dev); device_del(dev); } -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
> -Original Message- > From: James Bottomley [mailto:james.bottom...@hansenpartnership.com] > Sent: Thursday, May 22, 2014 7:38 AM > To: KY Srinivasan > Cc: Andy Whitcroft; Ian Abbott; linux-scsi@vger.kernel.org; > de...@linuxdriverproject.org; Haiyang Zhang; Tim Gardner > Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu > > On Thu, 2014-05-22 at 10:49 +, KY Srinivasan wrote: > > > > > > > -Original Message- > > > From: Andy Whitcroft [mailto:a...@canonical.com] > > > Sent: Wednesday, May 21, 2014 7:25 AM > > > To: Ian Abbott > > > Cc: linux-scsi@vger.kernel.org; de...@linuxdriverproject.org; KY > > > Srinivasan; Haiyang Zhang; James E.J. Bottomley; Tim Gardner > > > Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu > > > > > > On 16 May 2014 16:39, Ian Abbott wrote: > > > > These changes to the Microsoft Hyper-V storage driver in Ubuntu > > > > Saucy's > > > > 3.13 kernel look useful for the mainline kernel, especially as > > > > they enable 'TRIM' support. > > > > > > > > Andy Whitcroft (2): > > > > scsi: hyper-v storvsc switch up to SPC-3 > > > > scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly > -- > > > > elide them > > > > > > > > drivers/scsi/storvsc_drv.c | 10 ++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > > The back story here is a little complex. The main issue is that the > > > Hyper-V drives claim to be SPC-2, and yet implement the SPC-3 > extensions for TRIM. > > > We did attempt to upstream quirks to allow these features to be > > > negotiated specifically for the Hyper-V virtual drives (minimum > > > regression potential) but these were NAKd, and it was suggested that > > > just overriding the Hyper-V drives to SPC-3 unconditionally was more > > > appropriate. The first of the patches here does does this. This > > > has been sitting in our tree for some time as it was not clear that > > > this would be entirely safe, though the SPC-3 bits are in theory at > > > least mostly detected. That said this change has been in Ubuntu for > > > a full cycle now and does not seem to have caused any issues. If KY > > > is happy we should likely submit it formally. The second fix I believed > > > was > already being submitted to mainline. > > > > > > KY? > > > > The Windows guys are not currently comfortable claiming conformance to > > SPC-3, as they have not done the necessary testing. This will change > > hopefully soon. > > Any bounds on the value of "soon" are we talking weeks or months? I think > trim is a feature, which means no huge rush to get this in, but it is nice to > respond in a timely fashion to a request from a distribution to enable a > useful > feature. James, I know they have been testing for SPC-3 compliance. I have pinged them to see when they may give me the go ahead. I will keep you posted. Thanks, K. Y -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: dangling pointers and/or reentrancy in scmd_eh_abort_handler?
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Paolo Bonzini > Sent: Wednesday, 21 May, 2014 3:34 PM > To: Mark Wu; linux-scsi@vger.kernel.org > Subject: Re: dangling pointers and/or reentrancy in > scmd_eh_abort_handler? > > Il 21/05/2014 16:16, Mark Wu ha scritto: > > Is it possible that when scsi_done is invoked, the scsi command or the > > request has been freed and then reallocated for a new I/O request? > Because > > of this the bit flag REQ_ATOM_COMPLETE becomes unreliable. Thanks! > > It is up to the driver to ensure that the interrupt handler will not > process the Scsi_Cmnd* after returning from the eh_abort_handler. If > you do this, what you say cannot happen. Otherwise you'll get a variety > of failures, the most common of which for me are OOPSes and a BUG in > blk_finish_request. > > Paolo Aborts don't always work; scsi_eh_abort_handler cannot promise that it aborted the command (unless it escalated to sending resets.) When the abort fails, scmd_eh_abort_handler just leaves it outstanding, and the error handler thread (scsi_error_handler function) must deal with it. If the abort succeeds, then scmd_eh_abort_handler calls scsi_finish_command. In SCSI terms, FUNCTION COMPLETE for ABORT TASK doesn't mean the command was present and aborted and won't be sending back status; if the device server already sent status for the command, the task manager also sends FUNCTION COMPLETE. Depending on the transport, there may be a race condition between the command status and the ABORT TASK response; the ABORT TASK response might get back first. I think that means scsi_eh_abort_handler's call to scsi_finish_command could happen during or after scsi_softirq_done has called scsi_finish_command. --- Rob ElliottHP Server Storage -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] Drivers: scsi: storvsc: Change the limits to reflect the values on the host
Hyper-V hosts can support multiple targets and multiple channels and larger number of LUNs per target. Update the code to reflect this. With this patch we can correctly enumerate all the paths in a multi-path storage environment. MS-TFS: 173725 Signed-off-by: K. Y. Srinivasan Cc: --- drivers/scsi/storvsc_drv.c | 36 ++-- 1 files changed, 22 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 9969fa1..5a63772 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -331,16 +331,15 @@ static int storvsc_timeout = 180; static void storvsc_on_channel_callback(void *context); /* - * In Hyper-V, each port/path/target maps to 1 scsi host adapter. In - * reality, the path/target is not used (ie always set to 0) so our - * scsi host adapter essentially has 1 bus with 1 target that contains - * up to 256 luns. + * In Hyper-V, each port/path/target maps to 1 scsi host adapter. */ -#define STORVSC_MAX_LUNS_PER_TARGET64 -#define STORVSC_MAX_TARGETS1 -#define STORVSC_MAX_CHANNELS 1 - +#define STORVSC_MAX_LUNS_PER_TARGET255 +#define STORVSC_MAX_TARGETS2 +#define STORVSC_MAX_CHANNELS 8 +#define STORVSC_FC_MAX_LUNS_PER_TARGET 255 +#define STORVSC_FC_MAX_TARGETS 128 +#define STORVSC_FC_MAX_CHANNELS8 struct storvsc_cmd_request { struct list_head entry; @@ -1789,12 +1788,21 @@ static int storvsc_probe(struct hv_device *device, host_dev->path = stor_device->path_id; host_dev->target = stor_device->target_id; - /* max # of devices per target */ - host->max_lun = STORVSC_MAX_LUNS_PER_TARGET; - /* max # of targets per channel */ - host->max_id = STORVSC_MAX_TARGETS; - /* max # of channels */ - host->max_channel = STORVSC_MAX_CHANNELS - 1; + if (dev_id->driver_data == SFC_GUID) { + /* max # of devices per target */ + host->max_lun = STORVSC_FC_MAX_LUNS_PER_TARGET; + /* max # of targets per channel */ + host->max_id = STORVSC_FC_MAX_TARGETS; + /* max # of channels */ + host->max_channel = STORVSC_FC_MAX_CHANNELS - 1; + } else { + /* max # of devices per target */ + host->max_lun = STORVSC_MAX_LUNS_PER_TARGET; + /* max # of targets per channel */ + host->max_id = STORVSC_MAX_TARGETS; + /* max # of channels */ + host->max_channel = STORVSC_MAX_CHANNELS - 1; + } /* max cmd length */ host->max_cmd_len = STORVSC_MAX_CMD_LEN; -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH-scsi-for-next] tcm_qla2xxx: Set correct protection flags when DIF/DIX is enabled
From: Nicholas Bellinger With the advent of per session protection flags in v3.15-rc1 code, a target fabric driver supporting T10 PI needs to declare it's own protection capabilities at transport_init_session() time. This patch updates tcm_qla2xxx_check_initiator_node_acl() to extract the T10 PI capabilities using scsi_qla_host_t->flags.difdix_supported, and sets TARGET_PROT_ALL (all forms of PASS + INSERT + STRIP operations supported by HW fabric offloads) when available. Cc: Quinn Tran Cc: Giridhar Malavali Cc: Saurav Kashyap Cc: Christoph Hellwig Cc: James Bottomley Signed-off-by: Nicholas Bellinger --- drivers/scsi/qla2xxx/tcm_qla2xxx.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 896cb23..8c10438 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -1501,7 +1501,8 @@ static int tcm_qla2xxx_check_initiator_node_acl( struct qla_tgt_sess *sess = qla_tgt_sess; unsigned char port_name[36]; unsigned long flags; - + int prot_flags = (vha->flags.difdix_supported) ? TARGET_PROT_ALL : +TARGET_PROT_NORMAL; lport = vha->vha_tgt.target_lport_ptr; if (!lport) { pr_err("Unable to locate struct tcm_qla2xxx_lport\n"); @@ -1518,7 +1519,7 @@ static int tcm_qla2xxx_check_initiator_node_acl( } se_tpg = &tpg->se_tpg; - se_sess = transport_init_session(TARGET_PROT_NORMAL); + se_sess = transport_init_session(prot_flags); if (IS_ERR(se_sess)) { pr_err("Unable to initialize struct se_session\n"); return PTR_ERR(se_sess); -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 39/42] qla2xxx: ABTS cause double free of qla_tgt_cmd +.
Hi Saurav + Quinn, Just curious if this fix needs to be CC'ed to stable as well, or if it's something that is only triggered with the preceding T10 DIF support patch in place..? --nab On Fri, 2014-04-11 at 16:54 -0400, Saurav Kashyap wrote: > From: Quinn Tran > > Fix double free problem within qla2xxx driver where > current code prematurely free qla_tgt_cmd while firmware > still has the command. When firmware release the command > after abort, the code attempt a second free as part of > command completion processing. > > When TCM start the free process, NULL pointer was hit. > > -- > WARNING: CPU: 8 PID: 43613 at lib/list_debug.c:62 __list_del_entry+0x82/0xd0() > list_del corruption. next->prev should be 88082b5cfb08, but was > 6b6b6b6b6b6b6b6b > CPU: 8 PID: 43613 Comm: kworker/8:0 Tainted: GF W O > 3.13.0-rc3-nab_t10dif+ #6 > Hardware name: HP ProLiant DL380p Gen8, BIOS P70 08/20/2012 > Workqueue: events cache_reap > 003e 88081b2e3c78 815a051f 003e > 88081b2e3cc8 88081b2e3cb8 8104fc2c > 88082b5cfb00 88081c788d00 88082b5d7200 88082b5d3080 > Call Trace: > [] dump_stack+0x49/0x62 > [] warn_slowpath_common+0x8c/0xc0 > [] warn_slowpath_fmt+0x46/0x50 > [] __list_del_entry+0x82/0xd0 > [] process_one_work+0x12c/0x510 > [] ? process_one_work+0x173/0x510 > [] worker_thread+0x11f/0x3a0 > [] ? manage_workers+0x170/0x170 > [] kthread+0xf6/0x120 > [] ? __lock_release+0x133/0x1b0 > [] ? __init_kthread_worker+0x70/0x70 > [] ret_from_fork+0x7c/0xb0 > [] ? __init_kthread_worker+0x70/0x70 > ---[ end trace dfc05c3f7caf8ebe ]--- > BUG: unable to handle kernel NULL pointer dereference at 0008 > IP: [] process_one_work+0x31/0x510 > --- > > Signed-off-by: Quinn Tran > Signed-off-by: Giridhar Malavali > Signed-off-by: Saurav Kashyap > --- > drivers/scsi/qla2xxx/qla_target.c | 29 ++--- > 1 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_target.c > b/drivers/scsi/qla2xxx/qla_target.c > index f24d44c..b1d10f9 100644 > --- a/drivers/scsi/qla2xxx/qla_target.c > +++ b/drivers/scsi/qla2xxx/qla_target.c > @@ -2681,7 +2681,18 @@ static void qlt_send_term_exchange(struct > scsi_qla_host *vha, > rc = __qlt_send_term_exchange(vha, cmd, atio); > spin_unlock_irqrestore(&vha->hw->hardware_lock, flags); > done: > - if (rc == 1) { > + /* > + * Terminate exchange will tell fw to release any active CTIO > + * that's in FW posession and cleanup the exchange. > + * > + * "cmd->state == QLA_TGT_STATE_ABORTED" means CTIO is still > + * down at FW. Free the cmd later when CTIO comes back later > + * w/aborted(0x2) status. > + * > + * "cmd->state != QLA_TGT_STATE_ABORTED" means CTIO is already > + * back w/some err. Free the cmd now. > + */ > + if ((rc == 1) && (cmd->state != QLA_TGT_STATE_ABORTED)) { > if (!ha_locked && !in_interrupt()) > msleep(250); /* just in case */ > > @@ -2689,6 +2700,7 @@ done: > qlt_unmap_sg(vha, cmd); > vha->hw->tgt.tgt_ops->free_cmd(cmd); > } > + return; > } > > void qlt_free_cmd(struct qla_tgt_cmd *cmd) > @@ -2906,6 +2918,7 @@ static void qlt_do_ctio_completion(struct scsi_qla_host > *vha, uint32_t handle, > case CTIO_LIP_RESET: > case CTIO_TARGET_RESET: > case CTIO_ABORTED: > + /* driver request abort via Terminate exchange */ > case CTIO_TIMEOUT: > case CTIO_INVALID_RX_ID: > /* They are OK */ > @@ -2974,9 +2987,18 @@ static void qlt_do_ctio_completion(struct > scsi_qla_host *vha, uint32_t handle, > break; > } > > - if (cmd->state != QLA_TGT_STATE_NEED_DATA) > + > + /* "cmd->state == QLA_TGT_STATE_ABORTED" means > + * cmd is already aborted/terminated, we don't > + * need to terminate again. The exchange is already > + * cleaned up/freed at FW level. Just cleanup at driver > + * level. > + */ > + if ((cmd->state != QLA_TGT_STATE_NEED_DATA) && > + (cmd->state != QLA_TGT_STATE_ABORTED)) { > if (qlt_term_ctio_exchange(vha, ctio, cmd, status)) > return; > + } > } > skip_term: > > @@ -3007,7 +3029,8 @@ skip_term: > "not return a CTIO complete\n", vha->vp_idx, cmd->state); > } > > - if (unlikely(status != CTIO_SUCCESS)) { > + if (unlikely(status != CTIO_SUCCESS) && > + (cmd->state != QLA_TGT_STATE_ABORTED)) { > ql_dbg(ql_dbg_tgt_mgt, vha, 0xf01f, "Finishing failed CTIO\n"); > dump_stack(); > } -- To unsubscribe from this list: send the line "unsub
Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()
On 05/21/2014 03:30 PM, Bart Van Assche wrote: scmd->abort_work is only scheduled after the block layer has marked the request associated with a command as complete and for commands that are not on the eh_cmd_q list. A SCSI command is only requeued after the scmd->abort_work handler has started (requeueing clears the "complete" flag). This means that the cancel_delayed_work() statement in scsi_put_command() is a no-op. Hence remove it. Hmm. I've put in the cancel_delayed_work() as a safety guard, fully aware that it's one of the "this cannot happen" kind of things. But there is a workqueue and it might have elements on it. And when freeing a command we absolutely need to make sure that the workqueue is empty. So calling cancel_delayed_work() was the obvious thing to do. I'd be fine with adding a WARN_ON(!list_empty(&cmd->abort_work)) here, however. This will clear up the intent of this statement. Additionally, document how it is avoided that scsi_finish_command() and the SCSI error handler code are invoked concurrently for the same command via WARN_ON_ONCE() statements. This should avoid that the scsi error handler code confuses its readers. This I'd rather put into a separate patch, as it's really a different issue. Signed-off-by: Bart Van Assche Cc: Hannes Reinecke Cc: Paolo Bonzini Cc: Christoph Hellwig Cc: Jens Axboe Cc: Joe Lawrence --- block/blk-softirq.c | 6 ++ drivers/scsi/scsi.c | 2 -- drivers/scsi/scsi_error.c | 28 include/linux/blkdev.h| 1 + 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/block/blk-softirq.c b/block/blk-softirq.c index 53b1737..59bb52d 100644 --- a/block/blk-softirq.c +++ b/block/blk-softirq.c @@ -172,6 +172,12 @@ void blk_complete_request(struct request *req) } EXPORT_SYMBOL(blk_complete_request); +bool blk_rq_completed(struct request *rq) +{ + return test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags); +} +EXPORT_SYMBOL(blk_rq_completed); + static __init int blk_softirq_init(void) { int i; diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 88d46fe..04a282a 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -334,8 +334,6 @@ void scsi_put_command(struct scsi_cmnd *cmd) list_del_init(&cmd->list); spin_unlock_irqrestore(&cmd->device->list_lock, flags); - cancel_delayed_work(&cmd->abort_work); - __scsi_put_command(cmd->device->host, cmd); } EXPORT_SYMBOL(scsi_put_command); diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 14ce3b4..32a8cd1 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -108,6 +108,28 @@ static int scsi_host_eh_past_deadline(struct Scsi_Host *shost) return 1; } +static bool scmd_being_handled_in_other_context(struct scsi_cmnd *scmd) +{ + struct Scsi_Host *shost = scmd->device->host; + struct scsi_cmnd *c; + unsigned long flags; + bool ret = false; + + if (!blk_rq_completed(scmd->request)) + return true; + + spin_lock_irqsave(shost->host_lock, flags); + list_for_each_entry(c, &shost->eh_cmd_q, eh_entry) { + if (c == scmd) { + ret = true; + break; + } + } + spin_unlock_irqrestore(shost->host_lock, flags); + + return ret; +} + /** * scmd_eh_abort_handler - Handle command aborts * @work: command to be aborted. Can't we just check for !list_empty(&scmd->eh_entry) here? Should achieve the same with less computation... Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html