On 28.10.2015 23:06, Don Brace wrote:
> From: Scott Teel <scott.t...@pmcs.com>
>
> external array. So the driver must treat these differently from
> local LUNs when assigning lun/target.
>
> LUN's 'model' field has been used to detect Lun types that need
> special treatment, but the desire is to eliminate the need to reference
> specific array models, and support any external array.
>
> Pass-through RAID (PTRAID) luns are not luns of the local controller,
> so they are not reported in LUN count of command 'ID controller'.
> However, they ARE reported in "Report logical Luns" command.
> Local luns are listed first, then PTRAID LUNs.
>
> The number of luns from "Report LUNs" in excess of those reported by
> 'ID controller' are therefore the PTRAID LUNS.
>
> We can now remove function is_ext_target, and the 'white list'
> array of supported model names.
>
> Reviewed-by: Scott Teel <scott.t...@pmcs.com>
> Reviewed-by: Justin Lindley <justin.lind...@pmcs.com>
> Reviewed-by: Kevin Barnett <kevin.barn...@pmcs.com>
> Signed-off-by: Don Brace <don.br...@pmcs.com>
> ---
> drivers/scsi/hpsa.c | 141
> ++++++++++++++++++++++++++++++++++++++---------
> drivers/scsi/hpsa.h | 1
> drivers/scsi/hpsa_cmd.h | 11 ++++
> 3 files changed, 127 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 06207e2..11ea3e5 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -276,7 +276,6 @@ static int hpsa_scsi_ioaccel_queue_command(struct
> ctlr_info *h,
> static void hpsa_command_resubmit_worker(struct work_struct *work);
> static u32 lockup_detected(struct ctlr_info *h);
> static int detect_controller_lockup(struct ctlr_info *h);
> -static int is_ext_target(struct ctlr_info *h, struct hpsa_scsi_dev_t
> *device);
>
> static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev)
> {
> @@ -777,7 +776,7 @@ static ssize_t path_info_show(struct device *dev,
> hdev->bus, hdev->target, hdev->lun,
> scsi_device_type(hdev->devtype));
>
> - if (is_ext_target(h, hdev) ||
> + if (hdev->external ||
> hdev->devtype == TYPE_RAID ||
> is_logical_device(hdev)) {
> output_len += snprintf(path[i] + output_len,
> @@ -3037,6 +3036,35 @@ out:
> return rc;
> }
>
> +static int hpsa_bmic_id_controller(struct ctlr_info *h,
> + struct bmic_identify_controller *buf, size_t bufsize)
> +{
> + int rc = IO_OK;
> + struct CommandList *c;
> + struct ErrorInfo *ei;
> +
> + c = cmd_alloc(h);
> +
> + rc = fill_cmd(c, BMIC_IDENTIFY_CONTROLLER, h, buf, bufsize,
> + 0, RAID_CTLR_LUNID, TYPE_CMD);
> + if (rc)
> + goto out;
> +
> + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c,
> + PCI_DMA_FROMDEVICE, NO_TIMEOUT);
> + if (rc)
> + goto out;
> + ei = c->err_info;
> + if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
> + hpsa_scsi_interpret_error(h, c);
> + rc = -1;
> + }
> +out:
> + cmd_free(h, c);
> + return rc;
> +}
> +
> +
> static int hpsa_bmic_id_physical_device(struct ctlr_info *h,
> unsigned char scsi3addr[], u16 bmic_device_index,
> struct bmic_identify_physical_device *buf, size_t bufsize)
> @@ -3513,27 +3541,6 @@ static void hpsa_update_device_supports_aborts(struct
> ctlr_info *h,
> }
> }
>
> -static unsigned char *ext_target_model[] = {
> - "MSA2012",
> - "MSA2024",
> - "MSA2312",
> - "MSA2324",
> - "P2000 G3 SAS",
> - "MSA 2040 SAS",
> - NULL,
> -};
> -
> -static int is_ext_target(struct ctlr_info *h, struct hpsa_scsi_dev_t *device)
> -{
> - int i;
> -
> - for (i = 0; ext_target_model[i]; i++)
> - if (strncmp(device->model, ext_target_model[i],
> - strlen(ext_target_model[i])) == 0)
> - return 1;
> - return 0;
> -}
> -
> /*
> * Helper function to assign bus, target, lun mapping of devices.
> * Logical drive target and lun are assigned at this time, but
> @@ -3557,7 +3564,7 @@ static void figure_bus_target_lun(struct ctlr_info *h,
> return;
> }
> /* It's a logical device */
> - if (is_ext_target(h, device)) {
> + if (device->external) {
> hpsa_set_bus_target_lun(device,
> HPSA_EXTERNAL_RAID_VOLUME_BUS, (lunid >> 16) & 0x3fff,
> lunid & 0x00ff);
> @@ -3591,7 +3598,7 @@ static int add_ext_target_dev(struct ctlr_info *h,
> if (!is_logical_dev_addr_mode(lunaddrbytes))
> return 0; /* It's the logical targets that may lack lun 0. */
>
> - if (!is_ext_target(h, tmpdevice))
> + if (!tmpdevice->external)
> return 0; /* Only external target devices have this problem. */
>
> if (tmpdevice->lun == 0) /* if lun is 0, then we have a lun 0. */
> @@ -3650,6 +3657,29 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_info
> *h,
> return 0;
> }
>
> +static int figure_external_status(struct ctlr_info *h, int
> raid_ctlr_position,
> + int i, int nphysicals, int nlocal_logicals)
> +{
> + /* In report logicals, local logicals are listed first,
> + * then any externals.
> + */
> + int logicals_start = nphysicals + (raid_ctlr_position == 0);
> +
> + if (i == raid_ctlr_position)
> + return 0;
This^ is only to catch the case where (i == nphysicals + nlogicals),
so the last iteration in the outer for cycle?
> +
> + if (i < logicals_start)
> + return 0;
> +
> + /* i is in logicals range, but still within local logicals */
> + if ((i - nphysicals - (raid_ctlr_position == 0)) < nlocal_logicals)
Is this thea same as ((i - logicals_start) < nlocal_logicals) ?
the previous condition (i < logicals_start) looks as it were
a subset of this one and could be removed.
so a single
if ((i - logicals_start) < nlocal_logicals))
return 0;
would be sufficient?
The nlocal_logicals is in hpsa_update_scsi_devices an u32
to which you assign a '-1' in hpsa_set_local_logical_count,
here it's converted to an int , so it may be negative - is that intended?
-tm
> + return 0;
> +
> + return 1; /* it's an external lun */
> +}
> +
> +
> +
> /*
> * Do CISS_REPORT_PHYS and CISS_REPORT_LOG. Data is returned in physdev,
> * logdev. The number of luns in physdev and logdev are returned in
> @@ -3773,6 +3803,32 @@ static void hpsa_get_path_info(struct hpsa_scsi_dev_t
> *this_device,
> sizeof(this_device->bay));
> }
>
> +/* get number of local logical disks. */
> +static int hpsa_set_local_logical_count(struct ctlr_info *h,
> + struct bmic_identify_controller *id_ctlr,
> + u32 *nlocals)
> +{
> + int rc;
> +
> + if (!id_ctlr) {
> + dev_warn(&h->pdev->dev, "%s: id_ctlr buffer is NULL.\n",
> + __func__);
> + return -ENOMEM;
> + }
> + memset(id_ctlr, 0, sizeof(*id_ctlr));
> + rc = hpsa_bmic_id_controller(h, id_ctlr, sizeof(*id_ctlr));
> + if (!rc)
> + if (id_ctlr->configured_logical_drive_count < 256)
> + *nlocals = id_ctlr->configured_logical_drive_count;
> + else
> + *nlocals = le16_to_cpu(
> + id_ctlr->extended_logical_unit_count);
> + else
> + *nlocals = -1;
> + return rc;
> +}
> +
> +
> static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
> {
> /* the idea here is we could get notified
> @@ -3788,8 +3844,10 @@ static void hpsa_update_scsi_devices(struct ctlr_info
> *h, int hostno)
> struct ReportExtendedLUNdata *physdev_list = NULL;
> struct ReportLUNdata *logdev_list = NULL;
> struct bmic_identify_physical_device *id_phys = NULL;
> + struct bmic_identify_controller *id_ctlr = NULL;
> u32 nphysicals = 0;
> u32 nlogicals = 0;
> + u32 nlocal_logicals = 0;
> u32 ndev_allocated = 0;
> struct hpsa_scsi_dev_t **currentsd, *this_device, *tmpdevice;
> int ncurrent = 0;
> @@ -3803,9 +3861,10 @@ static void hpsa_update_scsi_devices(struct ctlr_info
> *h, int hostno)
> logdev_list = kzalloc(sizeof(*logdev_list), GFP_KERNEL);
> tmpdevice = kzalloc(sizeof(*tmpdevice), GFP_KERNEL);
> id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
> + id_ctlr = kzalloc(sizeof(*id_ctlr), GFP_KERNEL);
>
> if (!currentsd || !physdev_list || !logdev_list ||
> - !tmpdevice || !id_phys) {
> + !tmpdevice || !id_phys || !id_ctlr) {
> dev_err(&h->pdev->dev, "out of memory\n");
> goto out;
> }
> @@ -3819,6 +3878,13 @@ static void hpsa_update_scsi_devices(struct ctlr_info
> *h, int hostno)
> goto out;
> }
>
> + /* Set number of local logicals (non PTRAID) */
> + if (hpsa_set_local_logical_count(h, id_ctlr, &nlocal_logicals)) {
> + dev_warn(&h->pdev->dev,
> + "%s: Can't determine number of local logical
> devices.\n",
> + __func__);
> + }
> +
> /* We might see up to the maximum number of logical and physical disks
> * plus external target devices, and a device for the local RAID
> * controller.
> @@ -3883,6 +3949,11 @@ static void hpsa_update_scsi_devices(struct ctlr_info
> *h, int hostno)
> continue;
> }
>
> + /* Determine if this is a lun from an external target array */
> + tmpdevice->external =
> + figure_external_status(h, raid_ctlr_position, i,
> + nphysicals, nlocal_logicals);
> +
> figure_bus_target_lun(h, lunaddrbytes, tmpdevice);
> hpsa_update_device_supports_aborts(h, tmpdevice, lunaddrbytes);
> this_device = currentsd[ncurrent];
> @@ -3966,6 +4037,7 @@ out:
> kfree(currentsd);
> kfree(physdev_list);
> kfree(logdev_list);
> + kfree(id_ctlr);
> kfree(id_phys);
> }
>
> @@ -6418,6 +6490,23 @@ static int fill_cmd(struct CommandList *c, u8 cmd,
> struct ctlr_info *h,
> c->Request.CDB[7] = (size >> 16) & 0xFF;
> c->Request.CDB[8] = (size >> 8) & 0XFF;
> break;
> + case BMIC_IDENTIFY_CONTROLLER:
> + c->Request.CDBLen = 10;
> + c->Request.type_attr_dir =
> + TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
> + c->Request.Timeout = 0;
> + c->Request.CDB[0] = BMIC_READ;
> + c->Request.CDB[1] = 0;
> + c->Request.CDB[2] = 0;
> + c->Request.CDB[3] = 0;
> + c->Request.CDB[4] = 0;
> + c->Request.CDB[5] = 0;
> + c->Request.CDB[6] = BMIC_IDENTIFY_CONTROLLER;
> + c->Request.CDB[7] = (size >> 16) & 0xFF;
> + c->Request.CDB[8] = (size >> 8) & 0XFF;
> + c->Request.CDB[9] = 0;
> + break;
> +
> default:
> dev_warn(&h->pdev->dev, "unknown command 0x%c\n", cmd);
> BUG();
> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
> index 38d5534..0cf147b 100644
> --- a/drivers/scsi/hpsa.h
> +++ b/drivers/scsi/hpsa.h
> @@ -77,6 +77,7 @@ struct hpsa_scsi_dev_t {
> struct hpsa_scsi_dev_t *phys_disk[RAID_MAP_MAX_ENTRIES];
> int nphysical_disks;
> int supports_aborts;
> + int external; /* 1-from external array 0-not <0-unknown */
> };
>
> struct reply_queue_buffer {
> diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
> index c2c0737..c83eaf1 100644
> --- a/drivers/scsi/hpsa_cmd.h
> +++ b/drivers/scsi/hpsa_cmd.h
> @@ -286,6 +286,7 @@ struct SenseSubsystem_info {
> #define BMIC_FLASH_FIRMWARE 0xF7
> #define BMIC_SENSE_CONTROLLER_PARAMETERS 0x64
> #define BMIC_IDENTIFY_PHYSICAL_DEVICE 0x15
> +#define BMIC_IDENTIFY_CONTROLLER 0x11
>
> /* Command List Structure */
> union SCSI3Addr {
> @@ -682,6 +683,16 @@ struct hpsa_pci_info {
> u32 board_id;
> };
>
> +struct bmic_identify_controller {
> + u8 configured_logical_drive_count; /* offset 0 */
> + u8 pad1[153];
> + __le16 extended_logical_unit_count; /* offset 154 */
> + u8 pad2[136];
> + u8 controller_mode; /* offset 292 */
> + u8 pad3[32];
> +};
> +
> +
> struct bmic_identify_physical_device {
> u8 scsi_bus; /* SCSI Bus number on controller */
> u8 scsi_id; /* SCSI ID on this bus */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
