diff mbox

scsi: sd: Remember that READ CAPACITY(16) succeeded

Message ID 20180314161556.12860-1-martin.petersen@oracle.com (mailing list archive)
State Accepted
Headers show

Commit Message

Martin K. Petersen March 14, 2018, 4:15 p.m. UTC
The USB storage glue sets the try_rc_10_first flag in an attempt to
avoid wedging poorly implemented legacy USB devices.

If the device capacity is too large to be expressed in the provided
response buffer field of READ CAPACITY(10), a well-behaved device will
set the reported capacity to 0xFFFFFFFF. We will then attempt to issue a
READ CAPACITY(16) to obtain the real capacity.

Since this part of the discovery logic is not covered by the first_scan
flag, a warning will be printed a couple of times times per revalidate
attempt if we upgrade from READ CAPACITY(10) to READ CAPACITY(16).

Remember that we have successfully issued READ CAPACITY(16) so we can
take the fast path on subsequent revalidate attempts.

Reported-by: Menion <menion@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Laurence Oberman March 14, 2018, 4:47 p.m. UTC | #1
On Wed, 2018-03-14 at 12:15 -0400, Martin K. Petersen wrote:
> The USB storage glue sets the try_rc_10_first flag in an attempt to
> avoid wedging poorly implemented legacy USB devices.
> 
> If the device capacity is too large to be expressed in the provided
> response buffer field of READ CAPACITY(10), a well-behaved device
> will
> set the reported capacity to 0xFFFFFFFF. We will then attempt to
> issue a
> READ CAPACITY(16) to obtain the real capacity.
> 
> Since this part of the discovery logic is not covered by the
> first_scan
> flag, a warning will be printed a couple of times times per
> revalidate
> attempt if we upgrade from READ CAPACITY(10) to READ CAPACITY(16).
> 
> Remember that we have successfully issued READ CAPACITY(16) so we can
> take the fast path on subsequent revalidate attempts.
> 
> Reported-by: Menion <menion@gmail.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  drivers/scsi/sd.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index bff21e636ddd..6e971b94af7d 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2484,6 +2484,8 @@ sd_read_capacity(struct scsi_disk *sdkp,
> unsigned char *buffer)
>  				sector_size = old_sector_size;
>  				goto got_data;
>  			}
> +			/* Remember that READ CAPACITY(16) succeeded
> */
> +			sdp->try_rc_10_first = 0;
>  		}
>  	}
>  

Looks fine to me.
Reviewed-by: Laurence Oberman <loberman@redhat.com>
Menion March 22, 2018, 8:09 a.m. UTC | #2
Maybe is worth to add a log in KERN_NOTICE also

2018-03-14 17:47 GMT+01:00 Laurence Oberman <loberman@redhat.com>:
> On Wed, 2018-03-14 at 12:15 -0400, Martin K. Petersen wrote:
>> The USB storage glue sets the try_rc_10_first flag in an attempt to
>> avoid wedging poorly implemented legacy USB devices.
>>
>> If the device capacity is too large to be expressed in the provided
>> response buffer field of READ CAPACITY(10), a well-behaved device
>> will
>> set the reported capacity to 0xFFFFFFFF. We will then attempt to
>> issue a
>> READ CAPACITY(16) to obtain the real capacity.
>>
>> Since this part of the discovery logic is not covered by the
>> first_scan
>> flag, a warning will be printed a couple of times times per
>> revalidate
>> attempt if we upgrade from READ CAPACITY(10) to READ CAPACITY(16).
>>
>> Remember that we have successfully issued READ CAPACITY(16) so we can
>> take the fast path on subsequent revalidate attempts.
>>
>> Reported-by: Menion <menion@gmail.com>
>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>> ---
>>  drivers/scsi/sd.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index bff21e636ddd..6e971b94af7d 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -2484,6 +2484,8 @@ sd_read_capacity(struct scsi_disk *sdkp,
>> unsigned char *buffer)
>>                               sector_size = old_sector_size;
>>                               goto got_data;
>>                       }
>> +                     /* Remember that READ CAPACITY(16) succeeded
>> */
>> +                     sdp->try_rc_10_first = 0;
>>               }
>>       }
>>
>
> Looks fine to me.
> Reviewed-by: Laurence Oberman <loberman@redhat.com>
Bart Van Assche March 23, 2018, 3:39 a.m. UTC | #3
On Wed, 2018-03-14 at 12:15 -0400, Martin K. Petersen wrote:
> The USB storage glue sets the try_rc_10_first flag in an attempt to

> avoid wedging poorly implemented legacy USB devices.

> 

> If the device capacity is too large to be expressed in the provided

> response buffer field of READ CAPACITY(10), a well-behaved device will

> set the reported capacity to 0xFFFFFFFF. We will then attempt to issue a

> READ CAPACITY(16) to obtain the real capacity.

> 

> Since this part of the discovery logic is not covered by the first_scan

> flag, a warning will be printed a couple of times times per revalidate

> attempt if we upgrade from READ CAPACITY(10) to READ CAPACITY(16).

> 

> Remember that we have successfully issued READ CAPACITY(16) so we can

> take the fast path on subsequent revalidate attempts.


Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
diff mbox

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index bff21e636ddd..6e971b94af7d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2484,6 +2484,8 @@  sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer)
 				sector_size = old_sector_size;
 				goto got_data;
 			}
+			/* Remember that READ CAPACITY(16) succeeded */
+			sdp->try_rc_10_first = 0;
 		}
 	}