Configure number of LUs reported by 'report-luns'
v2 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.
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(-)
Signature · 1 line
1.7.11.7
PATCH v2 1/3 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
@@ -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
Re: [PATCH v2 1/3] Encapsulate scsi_do_report_luns
drivers/scsi/scsi_scan.c void int_to_scsilun(unsigned int lun, struct scsi_lun *scsilun) · line 1306
Quoted 7 lines · Rob Evers
drivers/scsi/scsi_scan.c +7
-0+/* +* 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.
Re: [PATCH v2 1/3] Encapsulate scsi_do_report_luns
Quoted 9 lines · Bart Van Assche
drivers/scsi/scsi_scan.c void int_to_scsilun(unsigned int lun, struct scsi_lun *scsilun) · line 1306
Quoted 7 lines · Rob Evers
drivers/scsi/scsi_scan.c +7
-0+/* +* 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 ;
Quoted 3 lines
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
PATCH v2 2/3 Configure reported luns
In reply to Rob EversChange 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
@@ -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 511static 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
Re: [PATCH v2 2/3] Configure reported luns
Quoted 2 lines · Rob Evers
length = ((data[0] << 24) | (data[1] << 16) | (data[2] << 8) | (data[3] << 0));
[ ... ]
drivers/scsi/scsi_scan.c static int scsi_report_lun_scan(struct scsi_target *starget, int bflags, · line 1491
Quoted 2 lines
drivers/scsi/scsi_scan.c +2
-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.
PATCH v2 3/3 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
@@ -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