Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1313951 > unrolled thread
| Started by | Alexander Duyck <aduyck@mirantis.com> |
|---|---|
| First post | 2016-01-21 07:40 +0100 |
| Last post | 2016-01-21 18:10 +0100 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/2] scsi: Do not attach VPD to devices that don't support it Alexander Duyck <aduyck@mirantis.com> - 2016-01-21 07:40 +0100
Re: [PATCH 1/2] scsi: Do not attach VPD to devices that don't support it Hannes Reinecke <hare@suse.de> - 2016-01-21 08:40 +0100
Re: [PATCH 1/2] scsi: Do not attach VPD to devices that don't support it Alexander Duyck <alexander.duyck@gmail.com> - 2016-01-21 18:10 +0100
| From | Alexander Duyck <aduyck@mirantis.com> |
|---|---|
| Date | 2016-01-21 07:40 +0100 |
| Subject | [PATCH 1/2] scsi: Do not attach VPD to devices that don't support it |
| Message-ID | <qTh4u-1mn-13@gated-at.bofh.it> |
The patch "scsi: rescan VPD attributes" introduced a regression in which
devices that don't support VPD were being scanned for VPD attributes
anyway. This could cause issues for this parts and should be avoided so
the check for scsi_level has been moved out of scsi_add_lun and into
scsi_attach_vpd so that all callers will not scan VPD for devices that
don't support it.
Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/scsi/scsi.c | 3 +++
drivers/scsi/scsi_scan.c | 3 +--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index b1bf42b93fcc..ed085e78c893 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -784,6 +784,9 @@ void scsi_attach_vpd(struct scsi_device *sdev)
int pg83_supported = 0;
unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
+ if (sdev->scsi_level < SCSI_3)
+ return;
+
if (sdev->skip_vpd_pages)
return;
retry_pg0:
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6a820668d442..1b16c89e0cf9 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -986,8 +986,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
}
}
- if (sdev->scsi_level >= SCSI_3)
- scsi_attach_vpd(sdev);
+ scsi_attach_vpd(sdev);
sdev->max_queue_depth = sdev->queue_depth;
[toc] | [next] | [standalone]
| From | Hannes Reinecke <hare@suse.de> |
|---|---|
| Date | 2016-01-21 08:40 +0100 |
| Subject | Re: [PATCH 1/2] scsi: Do not attach VPD to devices that don't support it |
| Message-ID | <qTi0x-20y-9@gated-at.bofh.it> |
| In reply to | #1313951 |
[Multipart message — attachments visible in raw view] — view raw
On 01/21/2016 07:35 AM, Alexander Duyck wrote:
> The patch "scsi: rescan VPD attributes" introduced a regression in which
> devices that don't support VPD were being scanned for VPD attributes
> anyway. This could cause issues for this parts and should be avoided so
> the check for scsi_level has been moved out of scsi_add_lun and into
> scsi_attach_vpd so that all callers will not scan VPD for devices that
> don't support it.
>
> Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> drivers/scsi/scsi.c | 3 +++
> drivers/scsi/scsi_scan.c | 3 +--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index b1bf42b93fcc..ed085e78c893 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -784,6 +784,9 @@ void scsi_attach_vpd(struct scsi_device *sdev)
> int pg83_supported = 0;
> unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
>
> + if (sdev->scsi_level < SCSI_3)
> + return;
> +
> if (sdev->skip_vpd_pages)
> return;
> retry_pg0:
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a820668d442..1b16c89e0cf9 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -986,8 +986,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
> }
> }
>
> - if (sdev->scsi_level >= SCSI_3)
> - scsi_attach_vpd(sdev);
> + scsi_attach_vpd(sdev);
>
> sdev->max_queue_depth = sdev->queue_depth;
>
>
Isn't this slightly pointless, given that we're testing the inverse
condition in scsi_attach_vpd()?
And in anycase, I guess we should be using the same logic sd.c is
using. Please see the attached patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +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)
[toc] | [prev] | [next] | [standalone]
| From | Alexander Duyck <alexander.duyck@gmail.com> |
|---|---|
| Date | 2016-01-21 18:10 +0100 |
| Message-ID | <qTqUb-8kQ-37@gated-at.bofh.it> |
| In reply to | #1313976 |
On Wed, Jan 20, 2016 at 11:37 PM, Hannes Reinecke <hare@suse.de> wrote:
> On 01/21/2016 07:35 AM, Alexander Duyck wrote:
>> The patch "scsi: rescan VPD attributes" introduced a regression in which
>> devices that don't support VPD were being scanned for VPD attributes
>> anyway. This could cause issues for this parts and should be avoided so
>> the check for scsi_level has been moved out of scsi_add_lun and into
>> scsi_attach_vpd so that all callers will not scan VPD for devices that
>> don't support it.
>>
>> Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>> drivers/scsi/scsi.c | 3 +++
>> drivers/scsi/scsi_scan.c | 3 +--
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index b1bf42b93fcc..ed085e78c893 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -784,6 +784,9 @@ void scsi_attach_vpd(struct scsi_device *sdev)
>> int pg83_supported = 0;
>> unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
>>
>> + if (sdev->scsi_level < SCSI_3)
>> + return;
>> +
>> if (sdev->skip_vpd_pages)
>> return;
>> retry_pg0:
>> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
>> index 6a820668d442..1b16c89e0cf9 100644
>> --- a/drivers/scsi/scsi_scan.c
>> +++ b/drivers/scsi/scsi_scan.c
>> @@ -986,8 +986,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>> }
>> }
>>
>> - if (sdev->scsi_level >= SCSI_3)
>> - scsi_attach_vpd(sdev);
>> + scsi_attach_vpd(sdev);
>>
>> sdev->max_queue_depth = sdev->queue_depth;
>>
>>
> Isn't this slightly pointless, given that we're testing the inverse
> condition in scsi_attach_vpd()?
I'm not sure what you are getting at. What I basically did is move
the check here into the function. No point in checking it in 2 spots
when checking it inside the function is good enough.
> And in anycase, I guess we should be using the same logic sd.c is
> using. Please see the attached patch.
The attached patch looks good as it also takes care of the opt-in case
which I had overlooked. The only bit missing is the fact that we are
still checking scsi_level twice when we don't need to.
- Alex
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web