Re: [usb-storage] USB storage devices and SAT

[Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>]

On Tue, 5 Aug 2008, Boaz Harrosh wrote:

> There was a correct and simple patch proposed that fixes this problem
> the right way:
> http://marc.info/?l=linux-usb&m=121762869915609&w=2
> 
> Doug could you please test this patch to see if it fixes your device?
> 
> scsi-core already gives drivers complete control on what sense_size to
> fetch. It is a parameter to the scsi_eh_prep_cmnd() call. So no need
> for slave_configure(), default value, and all that loop.

> > http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
> > Index: linux-2.6/drivers/usb/storage/scsiglue.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/usb/storage/scsiglue.c	2008-08-02 00:07:24.000000000 +0200
> > +++ linux-2.6/drivers/usb/storage/scsiglue.c	2008-08-02 00:07:37.000000000 +0200
> > @@ -170,6 +170,10 @@
> >  		if (us->fflags & US_FL_CAPACITY_HEURISTICS)
> >  			sdev->guess_capacity = 1;
> >  
> > +		/* assume SPC3 or latter device support sense size different of 18 */
> > +		if (sdev->scsi_level > SCSI_SPC_2)
> > +			us->fflags |= US_FL_SANE_SENSE;
> > +
> >  		/* Some devices report a SCSI revision level above 2 but are
> >  		 * unable to handle the REPORT LUNS command (for which
> >  		 * support is mandatory at level 3).  Since we already have
> > Index: linux-2.6/drivers/usb/storage/transport.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/usb/storage/transport.c	2008-08-02 00:07:24.000000000 \
> >                 +0200
> > +++ linux-2.6/drivers/usb/storage/transport.c	2008-08-02 00:07:37.000000000 +0200
> > @@ -595,10 +595,15 @@
> >  	if (need_auto_sense) {
> >  		int temp_result;
> >  		struct scsi_eh_save ses;
> > +		int sense_size = US_SENSE_SIZE;
> > +
> > +		/* device support and need bigger sense buffer */
> > +		if (us->fflags & US_FL_SANE_SENSE)
> > +			sense_size = ~0;

This looks highly suspicious at best.  Shouldn't sense_size be set to a
real value, like 22 or 255?


> I recommend this patch. It does exactly what's needed with minimum risk
> it is even maybe over protective, with the white list. Perhaps it should
> be turned to a black list instead. The old broken devices been an extincting
> exception.

Changing it to a blacklist would be bad -- in fact it would be a 
regression, because all the deficient devices which used to work would 
stop working until they were identified and added to the blacklist.

Apart from the one nit above, I agree that this patch looks sensible.

Alan Stern

--
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