Skip to contentSkip to conversation

Configure number of LUs reported by 'report-luns'

PATCH In review Rob Evers 5 participants 14 messages Mar 7, 2013 SCSI

lore
Share & tools
lore

Rob Evers

author
#0 lore raw

v3 cover Configure number of LUs reported by 'report-luns'

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(-)
Signature · 1 line
1.7.11.7
author
#1 lore raw

PATCH v3 1/4 Encapsulate scsi_do_report_luns

---
 drivers/scsi/scsi_scan.c | 109 +++++++++++++++++++++++++----------------------
 1 file changed, 59 insertions(+), 50 deletions(-)
drivers/scsi/scsi_scan.c +59 -50
index 3e58b22..b2abf22 100644
@@ -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) {
 		/*
Signature · 1 line
1.7.11.7
#2 lore raw

RE: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

Quoted 29 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Rob Evers
Sent: Thursday, 07 March, 2013 7:39 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

---
 drivers/scsi/scsi_scan.c | 109 +++++++++++++++++++++++++----------------------
 1 file changed, 59 insertions(+), 50 deletions(-)
drivers/scsi/scsi_scan.c +10 -0
index 3e58b22..b2abf22 100644
+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;
+

...

Quoted 16 lines
drivers/scsi/scsi_scan.c +15 -0
+	/*
+	 * 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,

drivers/scsi/scsi_scan.c void int_to_scsilun(unsigned int lun, struct scsi_lun *scsilun) · line 1327

Quoted 2 lines
drivers/scsi/scsi_scan.c +2 -0
+					  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

#3 lore raw

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

Quoted 10 lines · Robert (Server Storage)
Quoted 9 lines

+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);

Quoted 2 lines

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

#4 lore raw

RE: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

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:

Quoted 61 lines
drivers/scsi/scsi_scan.c +10 -1
+	/*
+	 * 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:[email protected]]
Sent: Thursday, 07 March, 2013 10:07 AM
To: Elliott, Robert (Server Storage)
Cc: Rob Evers; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

Quoted 2 lines · Robert (Server Storage)
Quoted 2 lines

+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

Quoted 4 lines
Quoted 4 lines

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

			  "
Quoted 1 line
Quoted 1 line

REPORT LUNS to %s (try %d)\n", devname, +

retries)); + result =

Quoted 1 line
Quoted 1 line

scsi_execute_req(sdev, scsi_cmd, DMA_FROM_DEVICE, +

lun_data,

Quoted 1 line
Quoted 1 line

length, &sshdr, + SCSI_TIMEOUT + 4 *

HZ, 3, NULL);

Quoted 3 lines

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

maintainer
#5 lore raw

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

Quoted 2 lines · Robert (Server Storage)

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

#6 lore raw

RE: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

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?

Quoted 27 lines

-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Thursday, 07 March, 2013 11:31 AM
To: Elliott, Robert (Server Storage)
Cc: Jeremy Linton; Rob Evers; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

Quoted 2 lines · Robert (Server Storage)

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

maintainer
#7 lore raw

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

Quoted 5 lines · 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?

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

#8 lore raw

Re: [PATCH V3 1/4] Encapsulate scsi_do_report_luns

In reply to James Bottomley

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

Quoted 4 lines · James Bottomley

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

author
#9 lore raw

PATCH v3 2/4 Configure reported luns

In reply to 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(-)
drivers/scsi/scsi_scan.c +68 -21
index b2abf22..671ff58 100644
@@ -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,
Signature · 1 line
1.7.11.7
author
#10 lore raw

PATCH v3 3/4 Change kmallocs in report_luns to use GFP_KERNEL

In reply to Rob Evers
---
 drivers/scsi/scsi_scan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/scsi_scan.c +2 -2
index 671ff58..1d41730 100644
@@ -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) {
Signature · 1 line
1.7.11.7
author
#11 lore raw

PATCH v3 4/4 Use set/get_unaligned_be32 in report_luns

In reply to Rob Evers
---
 drivers/scsi/scsi_scan.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
drivers/scsi/scsi_scan.c +4 -12
index 1d41730..31bda4b 100644
@@ -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));
 			}
 		}
Signature · 1 line
1.7.11.7
Acked-by on the v3 series
#12 lore raw
In reply to Rob Evers
Quoted 24 lines · 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()

Quoted 9 lines

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 <[email protected]>
author
#13 lore raw
In reply to Rob Evers
Quoted 24 lines · 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()

Quoted 9 lines

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 <[email protected]>