* [PATCH v2 0/3] Configure number of LUs reported by 'report-luns'
@ 2013-02-18 19:15 Rob Evers
2013-02-18 19:15 ` [PATCH v2 1/3] Encapsulate scsi_do_report_luns Rob Evers
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Rob Evers @ 2013-02-18 19:15 UTC (permalink / raw)
To: linux-scsi
This patch set retrieves the number of LUs available on a target
using the report-luns command. The initial size of the report-luns
command is 512 entries, as the previous default initial number was.
If more LUs than 511 are present on a target, the report-luns is
re-issued with the size indicated in the result of the original
report-luns, up to max_report_luns.
The default value of max_report_luns is increased to 16k-1 from 512-1.
2nd version changes from first posting:
- Minor tweak added in 2nd patch to use the number of luns
reported in the 2nd report-luns command, if it is executed.
There is a chance that the number changed between the
1st and 2nd report-luns.
- Add 3rd patch changing kmalloc flag in report luns from
GFP_ATOMIC to GFP_KERNEL, as this is more consistent with
the allocation flag in blk_alloc_queue_node()
Rob Evers (3):
Encapsulate scsi_do_report_luns
Configure reported luns
Change kmallocs in report_luns to use GFP_KERNEL
drivers/scsi/scsi_scan.c | 196 ++++++++++++++++++++++++++++++-----------------
1 file changed, 126 insertions(+), 70 deletions(-)
--
1.7.11.7
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] Encapsulate scsi_do_report_luns
2013-02-18 19:15 [PATCH v2 0/3] Configure number of LUs reported by 'report-luns' Rob Evers
@ 2013-02-18 19:15 ` Rob Evers
2013-02-19 7:15 ` Bart Van Assche
2013-02-18 19:15 ` [PATCH v2 2/3] Configure reported luns Rob Evers
2013-02-18 19:15 ` [PATCH v2 3/3] Change kmallocs in report_luns to use GFP_KERNEL Rob Evers
2 siblings, 1 reply; 7+ messages in thread
From: Rob Evers @ 2013-02-18 19:15 UTC (permalink / raw)
To: linux-scsi
Signed-off-by: Rob Evers <revers@redhat.com>
---
drivers/scsi/scsi_scan.c | 109 +++++++++++++++++++++++++----------------------
1 file changed, 59 insertions(+), 50 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3e58b22..b2abf22 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1282,6 +1282,64 @@ void int_to_scsilun(unsigned int lun, struct scsi_lun *scsilun)
}
EXPORT_SYMBOL(int_to_scsilun);
+int scsi_do_report_luns(struct scsi_device *sdev, int length,
+ struct scsi_lun *lun_data, char *devname)
+{
+ unsigned int retries;
+ unsigned char scsi_cmd[MAX_COMMAND_SIZE];
+ struct scsi_sense_hdr sshdr;
+ int result;
+
+ scsi_cmd[0] = REPORT_LUNS;
+
+ /*
+ * bytes 1 - 5: reserved, set to zero.
+ */
+ memset(&scsi_cmd[1], 0, 5);
+
+ /*
+ * bytes 6 - 9: length of the command.
+ */
+ scsi_cmd[6] = (unsigned char) (length >> 24) & 0xff;
+ scsi_cmd[7] = (unsigned char) (length >> 16) & 0xff;
+ scsi_cmd[8] = (unsigned char) (length >> 8) & 0xff;
+ scsi_cmd[9] = (unsigned char) length & 0xff;
+
+ scsi_cmd[10] = 0; /* reserved */
+ scsi_cmd[11] = 0; /* control */
+
+ /*
+ * We can get a UNIT ATTENTION, for example a power on/reset, so
+ * retry a few times (like sd.c does for TEST UNIT READY).
+ * Experience shows some combinations of adapter/devices get at
+ * least two power on/resets.
+ *
+ * Illegal requests (for devices that do not support REPORT LUNS)
+ * should come through as a check condition, and will not generate
+ * a retry.
+ */
+ for (retries = 0; retries < 3; retries++) {
+ SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: Sending"
+ " REPORT LUNS to %s (try %d)\n", devname,
+ retries));
+ result = scsi_execute_req(sdev, scsi_cmd, DMA_FROM_DEVICE,
+ lun_data, length, &sshdr,
+ SCSI_TIMEOUT + 4 * HZ, 3, NULL);
+
+ SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: REPORT LUNS"
+ " %s (try %d) result 0x%x\n", result
+ ? "failed" : "successful", retries, result));
+ if (result == 0)
+ break;
+ else if (scsi_sense_valid(&sshdr)) {
+ if (sshdr.sense_key != UNIT_ATTENTION)
+ break;
+ }
+ }
+
+ return result;
+}
+
/**
* scsi_report_lun_scan - Scan using SCSI REPORT LUN results
* @starget: which target
@@ -1306,15 +1364,12 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
int rescan)
{
char devname[64];
- unsigned char scsi_cmd[MAX_COMMAND_SIZE];
unsigned int length;
unsigned int lun;
unsigned int num_luns;
- unsigned int retries;
int result;
struct scsi_lun *lunp, *lun_data;
u8 *data;
- struct scsi_sense_hdr sshdr;
struct scsi_device *sdev;
struct Scsi_Host *shost = dev_to_shost(&starget->dev);
int ret = 0;
@@ -1369,53 +1424,7 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
goto out;
}
- scsi_cmd[0] = REPORT_LUNS;
-
- /*
- * bytes 1 - 5: reserved, set to zero.
- */
- memset(&scsi_cmd[1], 0, 5);
-
- /*
- * bytes 6 - 9: length of the command.
- */
- scsi_cmd[6] = (unsigned char) (length >> 24) & 0xff;
- scsi_cmd[7] = (unsigned char) (length >> 16) & 0xff;
- scsi_cmd[8] = (unsigned char) (length >> 8) & 0xff;
- scsi_cmd[9] = (unsigned char) length & 0xff;
-
- scsi_cmd[10] = 0; /* reserved */
- scsi_cmd[11] = 0; /* control */
-
- /*
- * We can get a UNIT ATTENTION, for example a power on/reset, so
- * retry a few times (like sd.c does for TEST UNIT READY).
- * Experience shows some combinations of adapter/devices get at
- * least two power on/resets.
- *
- * Illegal requests (for devices that do not support REPORT LUNS)
- * should come through as a check condition, and will not generate
- * a retry.
- */
- for (retries = 0; retries < 3; retries++) {
- SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "scsi scan: Sending"
- " REPORT LUNS to %s (try %d)\n", devname,
- retries));
-
- result = scsi_execute_req(sdev, scsi_cmd, DMA_FROM_DEVICE,
- lun_data, length, &sshdr,
- SCSI_TIMEOUT + 4 * HZ, 3, NULL);
-
- SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "scsi scan: REPORT LUNS"
- " %s (try %d) result 0x%x\n", result
- ? "failed" : "successful", retries, result));
- if (result == 0)
- break;
- else if (scsi_sense_valid(&sshdr)) {
- if (sshdr.sense_key != UNIT_ATTENTION)
- break;
- }
- }
+ result = scsi_do_report_luns(sdev, length, lun_data, devname);
if (result) {
/*
--
1.7.11.7
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] Configure reported luns
2013-02-18 19:15 [PATCH v2 0/3] Configure number of LUs reported by 'report-luns' Rob Evers
2013-02-18 19:15 ` [PATCH v2 1/3] Encapsulate scsi_do_report_luns Rob Evers
@ 2013-02-18 19:15 ` Rob Evers
2013-02-19 7:17 ` Bart Van Assche
2013-02-18 19:15 ` [PATCH v2 3/3] Change kmallocs in report_luns to use GFP_KERNEL Rob Evers
2 siblings, 1 reply; 7+ messages in thread
From: Rob Evers @ 2013-02-18 19:15 UTC (permalink / raw)
To: linux-scsi
Change default value of max_report_luns to 16k-1.
Use data returned from max report luns command to configure the number
of logical units present if previous default of 511 isn't enough.
Signed-off-by: Rob Evers <revers@redhat.com>
---
drivers/scsi/scsi_scan.c | 89 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 68 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index b2abf22..671ff58 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -109,12 +109,13 @@ MODULE_PARM_DESC(scan, "sync, async or none");
* in practice, the maximum number of LUNs suppored by any device
* is about 16k.
*/
-static unsigned int max_scsi_report_luns = 511;
+static unsigned int max_scsi_report_luns = 16383;
module_param_named(max_report_luns, max_scsi_report_luns, uint, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(max_report_luns,
"REPORT LUNS maximum number of LUNS received (should be"
- " between 1 and 16384)");
+ " between 1 and 16383)");
+#define INITIAL_MAX_REPORT_LUNS 511
static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ + 18;
@@ -1366,9 +1367,10 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
char devname[64];
unsigned int length;
unsigned int lun;
- unsigned int num_luns;
+ unsigned int num_luns, num_luns_reported;
int result;
struct scsi_lun *lunp, *lun_data;
+ struct scsi_lun *first_lun_data, *second_lun_data;
u8 *data;
struct scsi_device *sdev;
struct Scsi_Host *shost = dev_to_shost(&starget->dev);
@@ -1409,45 +1411,90 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
/*
* Allocate enough to hold the header (the same size as one scsi_lun)
* plus the max number of luns we are requesting.
- *
- * Reallocating and trying again (with the exact amount we need)
- * would be nice, but then we need to somehow limit the size
- * allocated based on the available memory and the limits of
- * kmalloc - we don't want a kmalloc() failure of a huge value to
- * prevent us from finding any LUNs on this target.
*/
- length = (max_scsi_report_luns + 1) * sizeof(struct scsi_lun);
- lun_data = kmalloc(length, GFP_ATOMIC |
- (sdev->host->unchecked_isa_dma ? __GFP_DMA : 0));
- if (!lun_data) {
+ if (max_scsi_report_luns > INITIAL_MAX_REPORT_LUNS)
+ length = (INITIAL_MAX_REPORT_LUNS + 1) *
+ sizeof(struct scsi_lun);
+ else
+ length = (max_scsi_report_luns + 1) *
+ sizeof(struct scsi_lun);
+
+ first_lun_data = kmalloc(length, GFP_ATOMIC |
+ (sdev->host->unchecked_isa_dma ?
+ __GFP_DMA : 0));
+ if (!first_lun_data) {
printk(ALLOC_FAILURE_MSG, __func__);
goto out;
}
- result = scsi_do_report_luns(sdev, length, lun_data, devname);
+ result = scsi_do_report_luns(sdev, length, first_lun_data, devname);
if (result) {
/*
* The device probably does not support a REPORT LUN command
*/
+ lun_data = first_lun_data;
ret = 1;
goto out_err;
}
/*
- * Get the length from the first four bytes of lun_data.
+ * Get the length from the first four bytes of first_lun_data.
*/
- data = (u8 *) lun_data->scsi_lun;
+ data = (u8 *) first_lun_data->scsi_lun;
length = ((data[0] << 24) | (data[1] << 16) |
(data[2] << 8) | (data[3] << 0));
- num_luns = (length / sizeof(struct scsi_lun));
- if (num_luns > max_scsi_report_luns) {
+ num_luns_reported = (length / sizeof(struct scsi_lun));
+
+ if (num_luns_reported > max_scsi_report_luns) {
+ num_luns = max_scsi_report_luns;
+ length = num_luns * sizeof(struct scsi_lun);
printk(KERN_WARNING "scsi: On %s only %d (max_scsi_report_luns)"
" of %d luns reported, try increasing"
- " max_scsi_report_luns.\n", devname,
- max_scsi_report_luns, num_luns);
- num_luns = max_scsi_report_luns;
+ " max_report_luns parameter.\n", devname,
+ max_scsi_report_luns, num_luns_reported);
+ } else {
+ num_luns = num_luns_reported;
+ }
+
+ if (num_luns > INITIAL_MAX_REPORT_LUNS) {
+ /*
+ * add one for the header
+ */
+ length = length + sizeof(struct scsi_lun);
+ second_lun_data = kmalloc(length, GFP_ATOMIC |
+ (sdev->host->unchecked_isa_dma ?
+ __GFP_DMA : 0));
+ if (!second_lun_data) {
+ num_luns = INITIAL_MAX_REPORT_LUNS;
+ lun_data = first_lun_data;
+ printk(ALLOC_FAILURE_MSG, __func__);
+ printk(KERN_WARNING "scsi: On %s %d of %d luns reported.\n",
+ devname, num_luns,
+ num_luns_reported);
+ } else {
+ kfree(first_lun_data);
+ lun_data = second_lun_data;
+ result = scsi_do_report_luns(sdev, length,
+ lun_data, devname);
+ if (result) {
+ ret = 1;
+ goto out_err;
+ } else {
+ /*
+ * Get the length from the first four bytes
+ * of second_lun_data.
+ */
+ data = (u8 *) lun_data->scsi_lun;
+ length = ((data[0] << 24) | (data[1] << 16) |
+ (data[2] << 8) | (data[3] << 0));
+
+ num_luns = (length / sizeof(struct scsi_lun));
+ }
+ }
+ } else {
+ lun_data = first_lun_data;
}
SCSI_LOG_SCAN_BUS(3, sdev_printk (KERN_INFO, sdev,
--
1.7.11.7
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] Change kmallocs in report_luns to use GFP_KERNEL
2013-02-18 19:15 [PATCH v2 0/3] Configure number of LUs reported by 'report-luns' Rob Evers
2013-02-18 19:15 ` [PATCH v2 1/3] Encapsulate scsi_do_report_luns Rob Evers
2013-02-18 19:15 ` [PATCH v2 2/3] Configure reported luns Rob Evers
@ 2013-02-18 19:15 ` Rob Evers
2 siblings, 0 replies; 7+ messages in thread
From: Rob Evers @ 2013-02-18 19:15 UTC (permalink / raw)
To: linux-scsi
Signed-off-by: Rob Evers <revers@redhat.com>
---
drivers/scsi/scsi_scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 671ff58..1d41730 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1419,7 +1419,7 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
length = (max_scsi_report_luns + 1) *
sizeof(struct scsi_lun);
- first_lun_data = kmalloc(length, GFP_ATOMIC |
+ first_lun_data = kmalloc(length, GFP_KERNEL |
(sdev->host->unchecked_isa_dma ?
__GFP_DMA : 0));
if (!first_lun_data) {
@@ -1463,7 +1463,7 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
* add one for the header
*/
length = length + sizeof(struct scsi_lun);
- second_lun_data = kmalloc(length, GFP_ATOMIC |
+ second_lun_data = kmalloc(length, GFP_KERNEL |
(sdev->host->unchecked_isa_dma ?
__GFP_DMA : 0));
if (!second_lun_data) {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] Encapsulate scsi_do_report_luns
2013-02-18 19:15 ` [PATCH v2 1/3] Encapsulate scsi_do_report_luns Rob Evers
@ 2013-02-19 7:15 ` Bart Van Assche
2013-02-19 15:57 ` Rob Evers
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2013-02-19 7:15 UTC (permalink / raw)
To: Rob Evers; +Cc: linux-scsi
On 02/18/13 20:15, Rob Evers wrote:
> + /*
> + * bytes 6 - 9: length of the command.
> + */
> + scsi_cmd[6] = (unsigned char) (length >> 24) & 0xff;
> + scsi_cmd[7] = (unsigned char) (length >> 16) & 0xff;
> + scsi_cmd[8] = (unsigned char) (length >> 8) & 0xff;
> + scsi_cmd[9] = (unsigned char) length & 0xff;
Have you considered using put_unaligned_be32() here ?
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] Configure reported luns
2013-02-18 19:15 ` [PATCH v2 2/3] Configure reported luns Rob Evers
@ 2013-02-19 7:17 ` Bart Van Assche
0 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2013-02-19 7:17 UTC (permalink / raw)
To: Rob Evers; +Cc: linux-scsi
On 02/18/13 20:15, Rob Evers wrote:
> length = ((data[0] << 24) | (data[1] << 16) |
> (data[2] << 8) | (data[3] << 0));
[ ... ]
> + length = ((data[0] << 24) | (data[1] << 16) |
> + (data[2] << 8) | (data[3] << 0));
Please consider using get_unaligned_be32() for these two statements.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] Encapsulate scsi_do_report_luns
2013-02-19 7:15 ` Bart Van Assche
@ 2013-02-19 15:57 ` Rob Evers
0 siblings, 0 replies; 7+ messages in thread
From: Rob Evers @ 2013-02-19 15:57 UTC (permalink / raw)
To: Bart Van Assche, James Bottomley; +Cc: linux-scsi
On 02/19/2013 02:15 AM, Bart Van Assche wrote:
> On 02/18/13 20:15, Rob Evers wrote:
>> + /*
>> + * bytes 6 - 9: length of the command.
>> + */
>> + scsi_cmd[6] = (unsigned char) (length >> 24) & 0xff;
>> + scsi_cmd[7] = (unsigned char) (length >> 16) & 0xff;
>> + scsi_cmd[8] = (unsigned char) (length >> 8) & 0xff;
>> + scsi_cmd[9] = (unsigned char) length & 0xff;
>
> Have you considered using put_unaligned_be32() here ?
>
> Bart.
Thanks for the suggestion Bart.
James,
Are you in favor of the type of change
Bart is suggesting?
I thought I saw some email a while back where
you disagreed with stylistic only changes to existing
code. Is that true and does it apply here?
Rob
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-19 15:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-18 19:15 [PATCH v2 0/3] Configure number of LUs reported by 'report-luns' Rob Evers
2013-02-18 19:15 ` [PATCH v2 1/3] Encapsulate scsi_do_report_luns Rob Evers
2013-02-19 7:15 ` Bart Van Assche
2013-02-19 15:57 ` Rob Evers
2013-02-18 19:15 ` [PATCH v2 2/3] Configure reported luns Rob Evers
2013-02-19 7:17 ` Bart Van Assche
2013-02-18 19:15 ` [PATCH v2 3/3] Change kmallocs in report_luns to use GFP_KERNEL Rob Evers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.