Re: kernel BUG in drivers/scsi/53c700.c:1129

[Helge Deller <deller@gmx.de>]

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@lst.de>
> 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@HansenPartnership.com
[    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
>