On 06/13/2016 03:50 PM, Ewan D. Milne wrote:
> On Mon, 2016-06-13 at 14:48 +0200, Hannes Reinecke wrote:
>> If we encounter an error during VPD page scanning we should be
>> setting the 'skip_vpd_pages' bit to avoid further accesses.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
>> ---
>> drivers/scsi/scsi.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index 1deb6ad..0359864 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -796,6 +796,7 @@ retry_pg0:
>> result = scsi_vpd_inquiry(sdev, vpd_buf, 0, vpd_len);
>> if (result < 0) {
>> kfree(vpd_buf);
>> + sdev->skip_vpd_pages = 1;
>> return;
>> }
>> if (result > vpd_len) {
>> @@ -822,6 +823,7 @@ retry_pg80:
>> result = scsi_vpd_inquiry(sdev, vpd_buf, 0x80, vpd_len);
>> if (result < 0) {
>> kfree(vpd_buf);
>> + sdev->skip_vpd_pages = 1;
>> return;
>> }
>> if (result > vpd_len) {
>> @@ -851,6 +853,7 @@ retry_pg83:
>> result = scsi_vpd_inquiry(sdev, vpd_buf, 0x83, vpd_len);
>> if (result < 0) {
>> kfree(vpd_buf);
>> + sdev->skip_vpd_pages = 1;
>> return;
>> }
>> if (result > vpd_len) {
>
> So, this changes scsi_attach_vpd() but not scsi_get_vpd_page() ?
>
Yes.
scsi_get_vpd_page() is just checking 'skip_vpd_pages', but none of the
other settings (ie it's not using scsi_device_supports_vpd()).
So we already come to different results when asking for VPD pages via
scsi_get_vpd_page() and scsi_attach_vpd().
Ideally we would be using scsi_device_supports_vpd() in both instances,
but that would require a further audit and I deemed it beyond the scope
of this patch.
> This particular implementation worries me. If we get an error
> performing a VPD inquiry, we will never, ever, attempt one again? What
> happens if a path is down at the time? The idea behind getting updated
> VPD info was that it might change, so if that does happen we don't want
> to stop updating after an isolated error.
>
> I think what we want to do is check if the VPD inquiry is supported on
> the *initial* inquiry, and if that fails then suppress further updates.
>
Fair point. Will be updating the patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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
