Re: SCSI regression in 4.11

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 2 Mar 2017 14:25:14 +0100
Hannes Reinecke <hare@suse.de> wrote:

> On 03/02/2017 02:40 AM, Stephen Hemminger wrote:
> > On Thu, 2 Mar 2017 01:56:15 +0100
> > Christoph Hellwig <hch@lst.de> wrote:
> >  
> >> On Thu, Mar 02, 2017 at 01:01:35AM +0100, Christoph Hellwig wrote:  
> >>> On Wed, Mar 01, 2017 at 07:54:12AM -0800, Stephen Hemminger wrote:  
> >>>>> 	http://git.infradead.org/users/hch/block.git/commitdiff/148cff67b401e2229c076c0ea418712654be77e4  
> >>>>
> >>>> It appears that is already in the code I am testing in linux-next...  
> >>>
> >>> It's in -next now, but it wasn't at the time you reported the bug.
> >>>
> >>> And it would sortof explain the bug if the INQUIRY data is correct
> >>> in the scatterlist, but we ignore it, given that scsi_probe_lun
> >>> ignores the result based on sense data.
> >>>
> >>> Can you check what happens with the horrible hack below:  
> >>
> >> Strike that - we're checking result later, so this can't be the case.
> >>
> >> Now the other interesting thing is the memset in __scsi_exectute,
> >> which looks very suspicious.  Try the following please:
> >>
> >> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> >> index 3e32dc954c3c..22f4fb550561 100644
> >> --- a/drivers/scsi/scsi_lib.c
> >> +++ b/drivers/scsi/scsi_lib.c
> >> @@ -253,7 +253,8 @@ static int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
> >>  	 * and prevent security leaks by zeroing out the excess data.
> >>  	 */
> >>  	if (unlikely(rq->resid_len > 0 && rq->resid_len <= bufflen))
> >> -		memset(buffer + (bufflen - rq->resid_len), 0, rq->resid_len);
> >> +//		memset(buffer + (bufflen - rq->resid_len), 0, rq->resid_len);
> >> +		printk_ratelimited("%s: got resid %d\n", __func__, rq->resid_len);
> >>
> >>  	if (resid)
> >>  		*resid = rq->resid_len;  
> >
> >
> > Still fails but does print resid on some of the later INQUIRY commands (not the initial one).
> >  
> Can you test what happens if you blank out the storvsc_drv workaround:
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 585e54f..c36f42d 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1060,13 +1060,13 @@ static void storvsc_on_io_completion(struct 
> storvsc_device *stor_device,
>           * We do this so we can distinguish truly fatal failues
>           * (srb status == 0x4) and off-line the device in that case.
>           */
> -
> +#if 0
>          if ((stor_pkt->vm_srb.cdb[0] == INQUIRY) ||
>             (stor_pkt->vm_srb.cdb[0] == MODE_SENSE)) {
>                  vstor_packet->vm_srb.scsi_status = 0;
>                  vstor_packet->vm_srb.srb_status = SRB_STATUS_SUCCESS;
>          }
> -
> +#endif
> 
>          /* Copy over the status...etc */
>          stor_pkt->vm_srb.scsi_status = vstor_packet->vm_srb.scsi_status;
> 
> It might thappen that we're fail to interpret the 'Device not present' 
> status correctly (which will happen for non-connected DVDs) causing the 
> SCSI stack to make incorrect decisions later on.
> 
> Cheers,
> 
> Hannes

There are several oddities about the host SCSI interface that I see:
  1. The host bus seems to report up to 6 devices even though only 2 are
     present (Disk and CDROM).
  2. The CDROM emulation doesn't report the same status as a real device.
  3. The host emulation of SCSI doesn't support all the page codes which
     is why there is the hack.

But as James said, these don't appear to be related to the failure because
the code worked before and only in post 4.11 merege is there a problem.