Hannes Reinecke wrote:
> Douglas Gilbert wrote:
>> Hannes Reinecke wrote:
>>> Douglas Gilbert wrote:
>>>> Hannes,
>>>> Here is my counter patch for you to consider.
>>>> I suspect my machine was running into stack
>>>> problems so I moved some arrays onto the heap.
>>>>
>>> Ah. Good. To tell the truth I was a bit concerned once I've
>>> discovered that all information pages are infact statically allocated
>>> ...
>>
>> I wasn't aware that statically allocated data (file or local
>> scope) would add to stack problems on a driver loaded as a
>> module. The arrays I moved to the heap were "auto" (i.e. non-static).
>>
> [ .. ]
>>
>> Over zealous error processing ....
>> I have made this mistake often enough, along with
>> many others, so hopefully some readers will take note:
>>
> I know. Been there before, too :-)
>
>> <snip>
> [ .. ]
>> So the "if (alloc_len < 4 + (11 * 2))" block should be
>> removed. Also the "if (alloc_len < n)" block should be
>> removed and "min" should be (possibly a cleaner version of):
>> min(min(alloc_len, n), SDEBUG_MAX_TGTPGS_ARR_SZ))
>>
> Ok, did so. And returned the proper (relative !) port number
> in the port descriptor list.
>
> Is this more to your liking?
Hannes,
Looks goods, tests well.
Signed-off-by: Douglas Gilbert <dougg@xxxxxxxxxx>
Doug Gilbert
> ------------------------------------------------------------------------
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 9c0f358..8e71fd2 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -52,7 +52,7 @@ #include "scsi_logging.h"
> #include "scsi_debug.h"
>
> #define SCSI_DEBUG_VERSION "1.80"
> -static const char * scsi_debug_version_date = "20060914";
> +static const char * scsi_debug_version_date = "20061018";
>
> /* Additional Sense Code (ASC) used */
> #define NO_ADDITIONAL_SENSE 0x0
> @@ -254,6 +254,8 @@ static int resp_requests(struct scsi_cmn
> struct sdebug_dev_info * devip);
> static int resp_start_stop(struct scsi_cmnd * scp,
> struct sdebug_dev_info * devip);
> +static int resp_report_tgtpgs(struct scsi_cmnd * scp,
> + struct sdebug_dev_info * devip);
> static int resp_readcap(struct scsi_cmnd * SCpnt,
> struct sdebug_dev_info * devip);
> static int resp_readcap16(struct scsi_cmnd * SCpnt,
> @@ -287,9 +289,9 @@ static void __init sdebug_build_parts(un
> static void __init init_all_queued(void);
> static void stop_all_queued(void);
> static int stop_queued_cmnd(struct scsi_cmnd * cmnd);
> -static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
> - int dev_id_num, const char * dev_id_str,
> - int dev_id_str_len);
> +static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
> + int target_dev_id, int dev_id_num,
> + const char * dev_id_str, int dev_id_str_len);
> static int inquiry_evpd_88(unsigned char * arr, int target_dev_id);
> static int do_create_driverfs_files(void);
> static void do_remove_driverfs_files(void);
> @@ -422,6 +424,15 @@ int scsi_debug_queuecommand(struct scsi_
> }
> errsts = resp_readcap16(SCpnt, devip);
> break;
> + case MAINTENANCE_IN:
> + if (MI_REPORT_TARGET_PGS != cmd[1]) {
> + mk_sense_buffer(devip, ILLEGAL_REQUEST,
> + INVALID_OPCODE, 0);
> + errsts = check_condition_result;
> + break;
> + }
> + errsts = resp_report_tgtpgs(SCpnt, devip);
> + break;
> case READ_16:
> case READ_12:
> case READ_10:
> @@ -665,8 +676,9 @@ static const char * inq_vendor_id = "Lin
> static const char * inq_product_id = "scsi_debug ";
> static const char * inq_product_rev = "0004";
>
> -static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
> - int dev_id_num, const char * dev_id_str,
> +static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
> + int target_dev_id, int dev_id_num,
> + const char * dev_id_str,
> int dev_id_str_len)
> {
> int num, port_a;
> @@ -720,6 +732,15 @@ static int inquiry_evpd_83(unsigned char
> arr[num++] = (port_a >> 16) & 0xff;
> arr[num++] = (port_a >> 8) & 0xff;
> arr[num++] = port_a & 0xff;
> + /* NAA-5, Target port group identifier */
> + arr[num++] = 0x61; /* proto=sas, binary */
> + arr[num++] = 0x95; /* piv=1, target port group id */
> + arr[num++] = 0x0;
> + arr[num++] = 0x4;
> + arr[num++] = 0;
> + arr[num++] = 0;
> + arr[num++] = (port_group_id >> 8) & 0xff;
> + arr[num++] = port_group_id & 0xff;
> /* NAA-5, Target device identifier */
> arr[num++] = 0x61; /* proto=sas, binary */
> arr[num++] = 0xa3; /* piv=1, target device, naa */
> @@ -928,12 +949,12 @@ static int resp_inquiry(struct scsi_cmnd
> struct sdebug_dev_info * devip)
> {
> unsigned char pq_pdt;
> - unsigned char arr[SDEBUG_MAX_INQ_ARR_SZ];
> + unsigned char * arr;
> unsigned char *cmd = (unsigned char *)scp->cmnd;
> - int alloc_len, n;
> + int alloc_len, n, ret;
>
> alloc_len = (cmd[3] << 8) + cmd[4];
> - memset(arr, 0, SDEBUG_MAX_INQ_ARR_SZ);
> + arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_KERNEL);
> if (devip->wlun)
> pq_pdt = 0x1e; /* present, wlun */
> else if (scsi_debug_no_lun_0 && (0 == devip->lun))
> @@ -944,12 +965,15 @@ static int resp_inquiry(struct scsi_cmnd
> if (0x2 & cmd[1]) { /* CMDDT bit set */
> mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
> 0);
> + kfree(arr);
> return check_condition_result;
> } else if (0x1 & cmd[1]) { /* EVPD bit set */
> - int lu_id_num, target_dev_id, len;
> + int lu_id_num, port_group_id, target_dev_id, len;
> char lu_id_str[6];
> int host_no = devip->sdbg_host->shost->host_no;
>
> + port_group_id = (((host_no + 1) & 0x7f) << 8) +
> + (devip->channel & 0x7f);
> if (0 == scsi_debug_vpd_use_hostno)
> host_no = 0;
> lu_id_num = devip->wlun ? -1 : (((host_no + 1) * 2000) +
> @@ -977,8 +1001,9 @@ static int resp_inquiry(struct scsi_cmnd
> memcpy(&arr[4], lu_id_str, len);
> } else if (0x83 == cmd[2]) { /* device identification */
> arr[1] = cmd[2]; /*sanity */
> - arr[3] = inquiry_evpd_83(&arr[4], target_dev_id,
> - lu_id_num, lu_id_str, len);
> + arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
> + target_dev_id, lu_id_num,
> + lu_id_str, len);
> } else if (0x84 == cmd[2]) { /* Software interface ident. */
> arr[1] = cmd[2]; /*sanity */
> arr[3] = inquiry_evpd_84(&arr[4]);
> @@ -1012,17 +1037,22 @@ static int resp_inquiry(struct scsi_cmnd
> /* Illegal request, invalid field in cdb */
> mk_sense_buffer(devip, ILLEGAL_REQUEST,
> INVALID_FIELD_IN_CDB, 0);
> + kfree(arr);
> return check_condition_result;
> }
> len = min(((arr[2] << 8) + arr[3]) + 4, alloc_len);
> - return fill_from_dev_buffer(scp, arr,
> + ret = fill_from_dev_buffer(scp, arr,
> min(len, SDEBUG_MAX_INQ_ARR_SZ));
> + kfree(arr);
> + return ret;
> }
> /* drops through here for a standard inquiry */
> arr[1] = DEV_REMOVEABLE(target) ? 0x80 : 0; /* Removable disk */
> arr[2] = scsi_debug_scsi_level;
> arr[3] = 2; /* response_data_format==2 */
> arr[4] = SDEBUG_LONG_INQ_SZ - 5;
> + if (0 == scsi_debug_vpd_use_hostno)
> + arr[5] = 0x10; /* claim: implicit TGPS */
> arr[6] = 0x10; /* claim: MultiP */
> /* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
> arr[7] = 0xa; /* claim: LINKED + CMDQUE */
> @@ -1039,8 +1069,10 @@ static int resp_inquiry(struct scsi_cmnd
> arr[n++] = 0x3; arr[n++] = 0x60; /* SSC-2 no version */
> }
> arr[n++] = 0xc; arr[n++] = 0xf; /* SAS-1.1 rev 10 */
> - return fill_from_dev_buffer(scp, arr,
> + ret = fill_from_dev_buffer(scp, arr,
> min(alloc_len, SDEBUG_LONG_INQ_SZ));
> + kfree(arr);
> + return ret;
> }
>
> static int resp_requests(struct scsi_cmnd * scp,
> @@ -1171,6 +1203,87 @@ static int resp_readcap16(struct scsi_cm
> min(alloc_len, SDEBUG_READCAP16_ARR_SZ));
> }
>
> +#define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
> +
> +static int resp_report_tgtpgs(struct scsi_cmnd * scp,
> + struct sdebug_dev_info * devip)
> +{
> + unsigned char *cmd = (unsigned char *)scp->cmnd;
> + unsigned char * arr;
> + int host_no = devip->sdbg_host->shost->host_no;
> + int n, ret, alen, rlen;
> + int port_group_a, port_group_b, port_a, port_b;
> +
> + alen = ((cmd[6] << 24) + (cmd[7] << 16) + (cmd[8] << 8)
> + + cmd[9]);
> +
> + arr = kzalloc(SDEBUG_MAX_TGTPGS_ARR_SZ, GFP_KERNEL);
> + /*
> + * EVPD page 0x88 states we have two ports, one
> + * real and a fake port with no device connected.
> + * So we create two port groups with one port each
> + * and set the group with port B to unavailable.
> + */
> + port_a = 0x1; /* relative port A */
> + port_b = 0x2; /* relative port B */
> + port_group_a = (((host_no + 1) & 0x7f) << 8) +
> + (devip->channel & 0x7f);
> + port_group_b = (((host_no + 1) & 0x7f) << 8) +
> + (devip->channel & 0x7f) + 0x80;
> +
> + /*
> + * The asymmetric access state is cycled according to the host_id.
> + */
> + n = 4;
> + if (0 == scsi_debug_vpd_use_hostno) {
> + arr[n++] = host_no % 3; /* Asymm access state */
> + arr[n++] = 0x0F; /* claim: all states are supported */
> + } else {
> + arr[n++] = 0x0; /* Active/Optimized path */
> + arr[n++] = 0x01; /* claim: only support active/optimized paths */
> + }
> + arr[n++] = (port_group_a >> 8) & 0xff;
> + arr[n++] = port_group_a & 0xff;
> + arr[n++] = 0; /* Reserved */
> + arr[n++] = 0; /* Status code */
> + arr[n++] = 0; /* Vendor unique */
> + arr[n++] = 0x1; /* One port per group */
> + arr[n++] = 0; /* Reserved */
> + arr[n++] = 0; /* Reserved */
> + arr[n++] = (port_a >> 8) & 0xff;
> + arr[n++] = port_a & 0xff;
> + arr[n++] = 3; /* Port unavailable */
> + arr[n++] = 0x08; /* claim: only unavailalbe paths are supported */
> + arr[n++] = (port_group_b >> 8) & 0xff;
> + arr[n++] = port_group_b & 0xff;
> + arr[n++] = 0; /* Reserved */
> + arr[n++] = 0; /* Status code */
> + arr[n++] = 0; /* Vendor unique */
> + arr[n++] = 0x1; /* One port per group */
> + arr[n++] = 0; /* Reserved */
> + arr[n++] = 0; /* Reserved */
> + arr[n++] = (port_b >> 8) & 0xff;
> + arr[n++] = port_b & 0xff;
> +
> + rlen = n - 4;
> + arr[0] = (rlen >> 24) & 0xff;
> + arr[1] = (rlen >> 16) & 0xff;
> + arr[2] = (rlen >> 8) & 0xff;
> + arr[3] = rlen & 0xff;
> +
> + /*
> + * Return the smallest value of either
> + * - The allocated length
> + * - The constructed command length
> + * - The maximum array size
> + */
> + rlen = min(alen,n);
> + ret = fill_from_dev_buffer(scp, arr,
> + min(rlen, SDEBUG_MAX_TGTPGS_ARR_SZ));
> + kfree(arr);
> + return ret;
> +}
> +
> /* <<Following mode page info copied from ST318451LW>> */
>
> static int resp_err_recov_pg(unsigned char * p, int pcontrol, int target)
> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index 84a6d5f..8a3f0bd 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -97,6 +97,7 @@ #define MODE_SENSE_10 0x5a
> #define PERSISTENT_RESERVE_IN 0x5e
> #define PERSISTENT_RESERVE_OUT 0x5f
> #define REPORT_LUNS 0xa0
> +#define MAINTENANCE_IN 0xa3
> #define MOVE_MEDIUM 0xa5
> #define EXCHANGE_MEDIUM 0xa6
> #define READ_12 0xa8
> @@ -114,6 +115,8 @@ #define VERIFY_16 0x8f
> #define SERVICE_ACTION_IN 0x9e
> /* values for service action in */
> #define SAI_READ_CAPACITY_16 0x10
> +/* values for maintenance in */
> +#define MI_REPORT_TARGET_PGS 0x0a
>
> /* Values for T10/04-262r7 */
> #define ATA_16 0x85 /* 16-byte pass-thru */
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
