> On Oct 28, 2015, at 5:05 PM, Don Brace <don.brace@xxxxxxxx> wrote:
>
> From: Kevin Barnett <kevin.barnett@xxxxxxxx>
>
> remove repeated calculation that checks for physical
> or logical devices.
>
> Reviewed-by: Scott Teel <scott.teel@xxxxxxxx>
> Reviewed-by: Justin Lindley <justin.lindley@xxxxxxxx>
> Reviewed-by: Kevin Barnett <kevin.barnett@xxxxxxxx>
> Signed-off-by: Don Brace <don.brace@xxxxxxxx>
> ---
> drivers/scsi/hpsa.c | 23 ++++++++++++++---------
> drivers/scsi/hpsa.h | 1 +
> 2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index d011540..7c1a552 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -3761,6 +3761,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
> int ncurrent = 0;
> int i, n_ext_target_devs, ndevs_to_allocate;
> int raid_ctlr_position;
> + bool physical_device;
Any particular reason for using a bool here and a u8 when you cache the value?
> DECLARE_BITMAP(lunzerobits, MAX_EXT_TARGETS);
>
> currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);
> @@ -3821,16 +3822,17 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
> int rc = 0;
> int phys_dev_index = i - (raid_ctlr_position == 0);
>
> + physical_device = i < nphysicals + (raid_ctlr_position == 0);
> +
> /* Figure out where the LUN ID info is coming from */
> lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,
> i, nphysicals, nlogicals, physdev_list, logdev_list);
>
> /* skip masked non-disk devices */
> - if (MASKED_DEVICE(lunaddrbytes))
> - if (i < nphysicals + (raid_ctlr_position == 0) &&
> - (physdev_list->
> - LUN[phys_dev_index].device_flags & 0x01))
> - continue;
> + if (physical_device &&
> + MASKED_DEVICE(lunaddrbytes) &&
> + (physdev_list->LUN[phys_dev_index].device_flags & 0x01))
> + continue;
In this conditional you swapped the ordering, evaluating physical_device first, why?
>
> /* Get device type, vendor, model, device id */
> rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice,
> @@ -3866,10 +3868,13 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
> }
>
> *this_device = *tmpdevice;
> + this_device->physical_device = physical_device;
>
> - /* do not expose masked devices */
> - if (MASKED_DEVICE(lunaddrbytes) &&
> - i < nphysicals + (raid_ctlr_position == 0))
> + /*
> + * Expose all devices except for physical devices that
> + * are masked.
> + */
> + if (MASKED_DEVICE(lunaddrbytes) && this_device->physical_device)
> this_device->expose_device = 0;
> else
> this_device->expose_device = 1;
> @@ -3887,7 +3892,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
> ncurrent++;
> break;
> case TYPE_DISK:
> - if (i < nphysicals + (raid_ctlr_position == 0)) {
> + if (this_device->physical_device) {
> /* The disk is in HBA mode. */
> /* Never use RAID mapper in HBA mode. */
> this_device->offload_enabled = 0;
> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
> index a6ead07..808b520 100644
> --- a/drivers/scsi/hpsa.h
> +++ b/drivers/scsi/hpsa.h
> @@ -37,6 +37,7 @@ struct hpsa_scsi_dev_t {
> unsigned int devtype;
> int bus, target, lun; /* as presented to the OS */
> unsigned char scsi3addr[8]; /* as presented to the HW */
> + u8 physical_device;
> u8 expose_device;
> #define RAID_CTLR_LUNID "\0\0\0\0\0\0\0\0"
> unsigned char device_id[16]; /* from inquiry pg. 0x83 */
>
> --
> 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
>
--
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
