From: Brian Bunker <brian@purestorage.com>
To: linux-scsi@vger.kernel.org
Cc: Brian Bunker <brian@purestorage.com>,
Seamus Connor <sconnor@purestorage.com>,
Krishna Kant <krishna.kant@purestorage.com>
Subject: [PATCH] scsi: sd: Avoid sending an INQUIRY if the page is not supported
Date: Fri, 5 May 2023 13:49:50 -0700 [thread overview]
Message-ID: <20230505204950.21645-1-brian@purestorage.com> (raw)
When SCSI devices are discovered the function sd_read_cpr gets called.
This call results in an INQUIRY to page 0xb9. This VPD page is called
regardless of whether the target has advertised this page as supported.
Instead of just sending this INQUIRY page, first check to see if that
page is in the supported pages. This will avoid sending requests to
targets which do not support the page. The error is unexpected on the
target and leads to questions. I am not sure what percentage of SCSI
devices support this page, but this will eliminate at least one
request to the target in the discovery phase for all that do not. The
function added could also have potential users besides this specific
one.
Signed-off-by: Brian Bunker <brian@purestorage.com>
Reviewed-by: Seamus Connor <sconnor@purestorage.com>
Reviewed-by: Krishna Kant <krishna.kant@purestorage.com>
---
drivers/scsi/scsi.c | 40 ++++++++++++++++++++++++++++++++++++++
drivers/scsi/sd.c | 4 +++-
include/scsi/scsi_device.h | 1 +
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 09ef0b31dfc0..9265b3d6a18f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -356,6 +356,46 @@ static int scsi_get_vpd_size(struct scsi_device *sdev, u8 page)
return result;
}
+/**
+ * scsi_vpd_page_supported - Check if a VPD page is supported on a SCSI device
+ * @sdev: The device to ask
+ * @page: Check existence of this Vital Product Data page
+ *
+ * Functions which explicitly request a given VPD page
+ * should first check whether that page is among the
+ * supported VPD pages. This will avoid targets returning
+ * unnecessary errors which can cause confusion. -EINVAL is
+ * returned if the page is not supported and 0 if it is.
+ */
+int scsi_vpd_page_supported(struct scsi_device *sdev, u8 page)
+{
+ const struct scsi_vpd *vpd;
+ uint16_t page_len;
+ int ret = -EINVAL;
+ int pos = 0;
+
+ rcu_read_lock();
+ vpd = rcu_dereference(sdev->vpd_pg0);
+ if (!vpd)
+ goto out;
+
+ page_len = get_unaligned_be16(&vpd->data[2]);
+
+ /*
+ * The first supported page starts at byte 4 in the buffer.
+ * Read from that byte to the last dictated by page_len above.
+ */
+ for (pos = 4; pos < page_len + 4; ++pos) {
+ if (vpd->data[pos] == page)
+ ret = 0;
+ }
+
+out:
+ rcu_read_unlock();
+ return ret;
+}
+EXPORT_SYMBOL_GPL(scsi_vpd_page_supported);
+
/**
* scsi_get_vpd_page - Get Vital Product Data from a SCSI device
* @sdev: The device to ask
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1624d528aa1f..0304b7d60747 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3127,7 +3127,9 @@ static void sd_read_cpr(struct scsi_disk *sdkp)
*/
buf_len = 64 + 256*32;
buffer = kmalloc(buf_len, GFP_KERNEL);
- if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb9, buffer, buf_len))
+ if (!buffer ||
+ scsi_vpd_page_supported(sdkp->device, 0xb9) ||
+ scsi_get_vpd_page(sdkp->device, 0xb9, buffer, buf_len))
goto out;
/* We must have at least a 64B header and one 32B range descriptor */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index f10a008e5bfa..359cd8b94312 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -431,6 +431,7 @@ extern int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
struct scsi_sense_hdr *);
extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
int retries, struct scsi_sense_hdr *sshdr);
+extern int scsi_vpd_page_supported(struct scsi_device *sdev, u8 page);
extern int scsi_get_vpd_page(struct scsi_device *, u8 page, unsigned char *buf,
int buf_len);
extern int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
--
2.40.1
next reply other threads:[~2023-05-05 20:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-05 20:49 Brian Bunker [this message]
2023-05-08 10:09 ` [PATCH] scsi: sd: Avoid sending an INQUIRY if the page is not supported Benjamin Block
2023-05-08 16:34 ` Brian Bunker
2023-05-11 16:44 ` Benjamin Block
2023-05-12 2:58 ` Douglas Gilbert
2023-05-08 12:26 ` Martin K. Petersen
2023-05-08 16:04 ` Brian Bunker
2023-05-10 14:29 ` Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230505204950.21645-1-brian@purestorage.com \
--to=brian@purestorage.com \
--cc=krishna.kant@purestorage.com \
--cc=linux-scsi@vger.kernel.org \
--cc=sconnor@purestorage.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox