[PATCH V3 0/4] Configure number of LUs reported by 'report-luns'

[PATCH V3 0/4] Configure number of LUs reported by 'report-luns'

[Rob Evers]

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.

3rd version changes from 2nd posting:

 - add a patch to use get/put_unaligned_be32() in report-luns code

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 (4):
  Encapsulate scsi_do_report_luns
  Configure reported luns
  Change kmallocs in report_luns to use GFP_KERNEL
  Use set/get_unaligned_be32 in report_luns

 drivers/scsi/scsi_scan.c | 192 +++++++++++++++++++++++++++++------------------
 1 file changed, 120 insertions(+), 72 deletions(-)

-- 
1.7.11.7

[PATCH V3 1/4] Encapsulate scsi_do_report_luns

[Rob Evers]

---
 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

RE: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[Elliott, Robert (Server Storage)]

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Rob Evers
> Sent: Thursday, 07 March, 2013 7:39 AM
> To: linux-scsi@vger.kernel.org
> Cc: revers@redhat.com; michaelc@cs.wisc.edu; bvanassche@acm.org;
> emilne@redhat.com
> Subject: [PATCH V3 1/4] Encapsulate scsi_do_report_luns
> 
> ---
>  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
> +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;
> +
...
> +	/*
> +	 * 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);

There's no guarantee that you'll get no more than two unit attention conditions at any particular time; a magic number of 3 retries isn't very robust.  Could this code retry until it stops getting CHECK CONDITION/UNIT ATTENTION?  It should include a much larger worst case number to avoid hangs if the device is truly stuck - maybe 20 times.  For CHECK CONDITION with other sense keys, an early exit is fine.

This may apply to other code too, since the comment mentions it is modeled after sd.c TEST UNIT READY handling.

---
Rob Elliott    HP Server Storage

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[Jeremy Linton]

On 3/7/2013 9:47 AM, Elliott, Robert (Server Storage) wrote:

>> +int scsi_do_report_luns(struct scsi_device *sdev, int length, +	 * 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. +	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);
> 
> There's no guarantee that you'll get no more than two unit attention
> conditions at any particular time;

	Actually, if your getting any unit attentions from a report luns the device
is broken. SAM5 5.14

"if a REPORT LUNS command enters the enabled command state, the device server
shall process the REPORTS LUNS command and shall not report any unit attention
conditions"

This is not new behavior either.


There are a couple other places that say similar things, INQUIRY and REPORT
LUNS get special status for UA. Which is how you can scan a target/lun
configuration without interfering with its operation. Personally, I think the
TUR in the mid layer is incorrect as the TUR functionality needs to be hoisted
higher up the stack and the mid layer needs to use inquiry to validate device
communications. (got a patch for that too, but no point in posting it, as it
will be ignored).

RE: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[Elliott, Robert (Server Storage)]

Good point; INQUIRY, REPORT LUNS, REQUEST SENSE, and NOTIFY DATA TRANSFER DEVICE do not report unit attention conditions.

The comment should be changed, since that's not the reason for any retries here:

> +	/*
> +	 * 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.
> +	 */


> -----Original Message-----
> From: Jeremy Linton [mailto:jlinton@tributary.com]
> Sent: Thursday, 07 March, 2013 10:07 AM
> To: Elliott, Robert (Server Storage)
> Cc: Rob Evers; linux-scsi@vger.kernel.org; michaelc@cs.wisc.edu;
> bvanassche@acm.org; emilne@redhat.com
> Subject: Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns
> 
> On 3/7/2013 9:47 AM, Elliott, Robert (Server Storage) wrote:
> 
> >> +int scsi_do_report_luns(struct scsi_device *sdev, int length, +	 * 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. +	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);
> >
> > There's no guarantee that you'll get no more than two unit attention
> > conditions at any particular time;
> 
> 	Actually, if your getting any unit attentions from a report luns the
> device
> is broken. SAM5 5.14
> 
> "if a REPORT LUNS command enters the enabled command state, the device
> server
> shall process the REPORTS LUNS command and shall not report any unit
> attention
> conditions"
> 
> This is not new behavior either.
> 
> 
> There are a couple other places that say similar things, INQUIRY and REPORT
> LUNS get special status for UA. Which is how you can scan a target/lun
> configuration without interfering with its operation. Personally, I think the
> TUR in the mid layer is incorrect as the TUR functionality needs to be hoisted
> higher up the stack and the mid layer needs to use inquiry to validate device
> communications. (got a patch for that too, but no point in posting it, as it
> will be ignored).
> 

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[James Bottomley]

On Thu, 2013-03-07 at 17:01 +0000, Elliott, Robert (Server Storage)
wrote:
> Good point; INQUIRY, REPORT LUNS, REQUEST SENSE, and NOTIFY DATA
> TRANSFER DEVICE do not report unit attention conditions.

Well, yes they do, at least on several devices I have here.

Can I point out again that we can't code to SAM ... we have to code to
what already exists. SAM is useful as a guideline, but it isn't gospel.
In particular where the real world does something SAM says it shouldn't
(like sending UA to INQUIRY), we have to go with the real world.

This also means we can't go through the linux SCSI subsystem changing
behaviour based on what SAM says the behaviour should be.  Most of what
the SCSI subsystem does is an accumulation based on years of trying to
fix it for annoying and out of spec devices.

James

RE: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[Elliott, Robert (Server Storage)]

So far the T10 Base feature set proposal has not been restating requirements already required by the core standards - it's just upgrading "mays" and "shoulds" to "shalls".  Should we also include a list of "shall" rules like this that have had known violations in the past?


> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Thursday, 07 March, 2013 11:31 AM
> To: Elliott, Robert (Server Storage)
> Cc: Jeremy Linton; Rob Evers; linux-scsi@vger.kernel.org;
> michaelc@cs.wisc.edu; bvanassche@acm.org; emilne@redhat.com
> Subject: Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns
> 
> On Thu, 2013-03-07 at 17:01 +0000, Elliott, Robert (Server Storage)
> wrote:
> > Good point; INQUIRY, REPORT LUNS, REQUEST SENSE, and NOTIFY DATA
> > TRANSFER DEVICE do not report unit attention conditions.
> 
> Well, yes they do, at least on several devices I have here.
> 
> Can I point out again that we can't code to SAM ... we have to code to
> what already exists. SAM is useful as a guideline, but it isn't gospel.
> In particular where the real world does something SAM says it shouldn't
> (like sending UA to INQUIRY), we have to go with the real world.
> 
> This also means we can't go through the linux SCSI subsystem changing
> behaviour based on what SAM says the behaviour should be.  Most of what
> the SCSI subsystem does is an accumulation based on years of trying to
> fix it for annoying and out of spec devices.
> 
> James
> 

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[James Bottomley]

On Thu, 2013-03-07 at 17:38 +0000, Elliott, Robert (Server Storage)
wrote:
> So far the T10 Base feature set proposal has not been restating
> requirements already required by the core standards - it's just
> upgrading "mays" and "shoulds" to "shalls".  Should we also include a
> list of "shall" rules like this that have had known violations in the
> past?

You mean trying to catalogue known broken behaviour?  That's a pretty
monumental task, particularly when you have to deal with all the USB
SCSI implementations, which is where a lot of our current violations
come from.

James

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

[Jeremy Linton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 3/7/2013 11:30 AM, James Bottomley wrote:

> This also means we can't go through the linux SCSI subsystem changing 
> behaviour based on what SAM says the behaviour should be.  Most of what the
> SCSI subsystem does is an accumulation based on years of trying to fix it
> for annoying and out of spec devices.

	Well, I wasn't suggesting removing the retries for this patch, cause yes
there are a lot of non complaint devices, but I was complaining about a case
where there are known problems with the way the code is executing on non
broken devices.

	Basically, prioritizing the functionality of a broken device, of the
functionality of a working one.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRONGAAAoJEL5i86xrzcy7a6QH/16EQyMQ3DzLrX2a3OdtSD4Q
QdHInok1SAyGKDGHTHXGu0RKuvpzgdSLjORKfEdbok/ZyNXd7qSi57czRV7R5U4b
nTLoaP8maXxJsJ1ko11sTEfZNT4cgO4+hLMjcZk9LBJZhNC+WqsszYaOVVLFtSIJ
xpBaowSjxLpkhi5cTdZ6p4+Tr2xgZxBXd+5NUZuB1s6ZJ99yNYcn97Q/3VVeFmW9
sprBP3kkiWv3LOIN6ZNTkKRDtgJYzf2LVTogjtNfCQsB/ZUHr5ITzZ1fMBkVrR7c
yVe4kdq26RDC57oSJMqAHA8QXBQ2ll8l8fz1X1mebb2TeyOI57/U8ZbPyfGvzxo=
=yZvD
-----END PGP SIGNATURE-----

[PATCH V3 2/4] Configure reported luns

[Rob Evers]

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.
---
 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

[PATCH V3 3/4] Change kmallocs in report_luns to use GFP_KERNEL

[Rob Evers]

---
 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

[PATCH V3 4/4] Use set/get_unaligned_be32 in report_luns

[Rob Evers]

---
 drivers/scsi/scsi_scan.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 1d41730..31bda4b 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -34,6 +34,7 @@
 #include <linux/spinlock.h>
 #include <linux/async.h>
 #include <linux/slab.h>
+#include <asm/unaligned.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1301,10 +1302,7 @@ int scsi_do_report_luns(struct scsi_device *sdev, int length,
 	/*
 	 * 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;
+	put_unaligned_be32(0xffffffff, &scsi_cmd[6]);
 
 	scsi_cmd[10] = 0;	/* reserved */
 	scsi_cmd[11] = 0;	/* control */
@@ -1441,10 +1439,7 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
 	/*
 	 * Get the length from the first four bytes of first_lun_data.
 	 */
-	data = (u8 *) first_lun_data->scsi_lun;
-	length = ((data[0] << 24) | (data[1] << 16) |
-		  (data[2] << 8) | (data[3] << 0));
-
+	length = get_unaligned_be32(first_lun_data->scsi_lun);
 	num_luns_reported = (length / sizeof(struct scsi_lun));
 
 	if (num_luns_reported > max_scsi_report_luns) {
@@ -1486,10 +1481,7 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
 				 * 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));
-
+				length = get_unaligned_be32(lun_data->scsi_lun);
 				num_luns = (length / sizeof(struct scsi_lun));
 			}
 		}
-- 
1.7.11.7

Re: [PATCH V3 0/4] Configure number of LUs reported by 'report-luns'

[Ewan Milne]

On Thu, 2013-03-07 at 08:38 -0500, Rob Evers wrote:
> 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.
> 
> 3rd version changes from 2nd posting:
> 
>  - add a patch to use get/put_unaligned_be32() in report-luns code
> 
> 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 (4):
>   Encapsulate scsi_do_report_luns
>   Configure reported luns
>   Change kmallocs in report_luns to use GFP_KERNEL
>   Use set/get_unaligned_be32 in report_luns
> 
>  drivers/scsi/scsi_scan.c | 192 +++++++++++++++++++++++++++++------------------
>  1 file changed, 120 insertions(+), 72 deletions(-)
> 

For all 4 patches in the series:

Acked-by: Ewan D. Milne <emilne@redhat.com>

Re: [PATCH V3 0/4] Configure number of LUs reported by 'report-luns'

[Rob Evers]

On 03/07/2013 08:38 AM, Rob Evers wrote:
> 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.
>
> 3rd version changes from 2nd posting:
>
>   - add a patch to use get/put_unaligned_be32() in report-luns code
>
> 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 (4):
>    Encapsulate scsi_do_report_luns
>    Configure reported luns
>    Change kmallocs in report_luns to use GFP_KERNEL
>    Use set/get_unaligned_be32 in report_luns
>
>   drivers/scsi/scsi_scan.c | 192 +++++++++++++++++++++++++++++------------------
>   1 file changed, 120 insertions(+), 72 deletions(-)
>

All patches in series Signed-off-by: Rob Evers <revers@redhat.com>