On 10.06.2016 23:33, James Bottomley wrote:
> On Fri, 2016-06-10 at 14:01 -0700, James Bottomley wrote:
>> On Fri, 2016-06-10 at 16:58 -0400, Ewan D. Milne wrote:
>>> I'm not sure if this is the problem, but the tagging changes to
>>> scsi_tcq.h may have altered the 53c700 driver's assumptions.
>>> In one case it sets sdev->current_cmnd and then some of the
>>> tagging calls would return it if the tag was SCSI_NO_TAG.
>>>
>>> NCR_700_queuecommand_lck() does:
>>>
>>> if ((hostdata->tag_negotiated & (1<<scmd_id(SCp))) &&
>>> SCp->device->simple_tags) {
>>> slot->tag = SCp->request->tag;
>>> CDEBUG(KERN_DEBUG, SCp, "sending out tag %d, slot
>>> %p\n",
>>> slot->tag, slot);
>>> } else {
>>> slot->tag = SCSI_NO_TAG;
>>> /* must populate current_cmnd for
>>> scsi_host_find_tag
>>> to
>>> work */
>>> SCp->device->current_cmnd = SCp;
>>> }
>>
>> Thanks ... I was just about to look for something this. I'd got to
>> interpreting the script as reselected with tag information present
>> and then trying to look the command up with no tag present, hence the
>> BUG().
>
> Yes, you're right, it's
>
> commit 64d513ac31bd02a3c9b69ef04444f36c196f9a9d
> Author: Christoph Hellwig <hch@xxxxxx>
> Date: Thu Oct 8 09:28:04 2015 +0100
>
> scsi: use host wide tags by default
>
> Again. This time because it's transformation of the handling of
> SCSI_NO_TAG is wrong. You can't replace the return sdev->current_cmnd
> original in scsi_find_tag with the NULL return in scsi_find_host_tag.
>
> I think this changesets wins the prize for the greatest number of
> generated faults.
>
> Does this fix 53c700.c?
Yes, the patch below fixes the boot problems for 53c700.c:
[ 9.869236] 53c700: Version 2.8 By James.Bottomley@xxxxxxxxxxxxxxxxxxxxx
[ 9.949797] scsi0: 53c710 rev 2
[ 10.994453] scsi host0: LASI SCSI 53c700
[ 13.008861] scsi 0:0:6:0: Direct-Access QUANTUM FIREBALL_TM3200S 300X PQ: 0 ANSI: 2
[ 13.106740] scsi target0:0:6: Beginning Domain Validation
[ 13.171889] scsi 0:0:6:0: tag#0 Enabling Tag Command Queuing
[ 13.239996] scsi target0:0:6: asynchronous
[ 13.305374] scsi target0:0:6: FAST-10 SCSI 10.0 MB/s ST (100 ns, offset 8)
[ 13.397482] scsi target0:0:6: Domain Validation skipping write tests
[ 13.473831] scsi target0:0:6: Ending Domain Validation
[ 13.557116] scsi 0:0:6:1: tag#0 Disabling Tag Command Queuing
[ 13.637379] st: Version 20160209, fixed bufsize 32768, s/g segs 256
[ 13.746803] sd 0:0:6:0: Attached scsi generic sg0 type 0
THANKS!
(please queue the patch up for 4.6-stable at least)
Helge
> I suppose we'd better look for other places where no tag fell through
> ...
>
> James
>
> ---
>
> diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
> index d4c2856..3ddc85e 100644
> --- a/drivers/scsi/53c700.c
> +++ b/drivers/scsi/53c700.c
> @@ -1122,7 +1122,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
> } else {
> struct scsi_cmnd *SCp;
>
> - SCp = scsi_host_find_tag(SDp->host, SCSI_NO_TAG);
> + SCp = SDp->current_cmnd;
> if(unlikely(SCp == NULL)) {
> sdev_printk(KERN_ERR, SDp,
> "no saved request for untagged cmd\n");
> @@ -1826,7 +1826,7 @@ NCR_700_queuecommand_lck(struct scsi_cmnd *SCp, void (*done)(struct scsi_cmnd *)
> slot->tag, slot);
> } else {
> slot->tag = SCSI_NO_TAG;
> - /* must populate current_cmnd for scsi_host_find_tag to work */
> + /* save current command for reselection */
> SCp->device->current_cmnd = SCp;
> }
> /* sanity check: some of the commands generated by the mid-layer
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
