* [PATCH 00/13] hpsa update
@ 2014-11-14 23:26 Don Brace
2014-11-14 23:26 ` [PATCH 01/13] hpsa: Clean up warnings from sparse Don Brace
` (13 more replies)
0 siblings, 14 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
This patch set is based on Linus's tree.
The changes are:
- correct warnings from sparse
- updates for some error handling issues
- general code cleanup
- performance enhancements based on removing spin_locks
---
Don Brace (1):
hpsa: Clean up warnings from sparse.
Nicholas Bellinger (1):
hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation
Robert Elliott (2):
hpsa: remove dev_warn prints from RAID-1ADM
hpsa: always call pci_set_master after pci_enable_device
Stephen M. Cameron (8):
hpsa: fix a couple pci id table mistakes
hpsa: remove 'action required' phrasing
hpsa: fix allocation sizes for CISS_REPORT_LUNs commands
hpsa: fix endianness issue with scatter gather elements
hpsa: get rid of type/attribute/direction bit field where possible
hpsa: use atomics for commands_outstanding
hpsa: do not be so noisy about check conditions
hpsa: remove spin lock around command allocation
Webb Scales (1):
hpsa: correct off-by-one sizing of chained SG block
drivers/scsi/hpsa.c | 488 ++++++++++++++++++++---------------------------
drivers/scsi/hpsa.h | 33 +--
drivers/scsi/hpsa_cmd.h | 34 ++-
3 files changed, 234 insertions(+), 321 deletions(-)
--
Don Brace
don.brace@pmcs.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 01/13] hpsa: Clean up warnings from sparse.
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:26 ` [PATCH 02/13] hpsa: remove dev_warn prints from RAID-1ADM Don Brace
` (12 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
Clean up issues reported when running sparse.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Webb Scales <webb.scales@hp.com>
---
drivers/scsi/hpsa.c | 29 ++++++++++++++++-------------
drivers/scsi/hpsa.h | 6 +++---
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index cef5d49..f345cc5 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -48,6 +48,7 @@
#include <linux/bitmap.h>
#include <linux/atomic.h>
#include <linux/jiffies.h>
+#include <linux/percpu-defs.h>
#include <linux/percpu.h>
#include <asm/div64.h>
#include "hpsa_cmd.h"
@@ -193,12 +194,13 @@ static int number_of_controllers;
static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void *arg);
+static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
static void lock_and_start_io(struct ctlr_info *h);
static void start_io(struct ctlr_info *h, unsigned long *flags);
#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void *arg);
+static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+ void __user *arg);
#endif
static void cmd_free(struct ctlr_info *h, struct CommandList *c);
@@ -2941,8 +2943,8 @@ static int hpsa_gather_lun_info(struct ctlr_info *h,
return 0;
}
-u8 *figure_lunaddrbytes(struct ctlr_info *h, int raid_ctlr_position, int i,
- int nphysicals, int nlogicals,
+static u8 *figure_lunaddrbytes(struct ctlr_info *h, int raid_ctlr_position,
+ int i, int nphysicals, int nlogicals,
struct ReportExtendedLUNdata *physdev_list,
struct ReportLUNdata *logdev_list)
{
@@ -4410,7 +4412,7 @@ static struct CommandList *hpsa_find_cmd_in_queue(struct ctlr_info *h,
struct CommandList *c = NULL; /* ptr into cmpQ */
if (!find)
- return 0;
+ return NULL;
spin_lock_irqsave(&h->lock, flags);
list_for_each_entry(c, queue_head, list) {
if (c->scsi_cmd == NULL) /* e.g.: passthru ioctl */
@@ -4785,7 +4787,8 @@ static void cmd_special_free(struct ctlr_info *h, struct CommandList *c)
#ifdef CONFIG_COMPAT
-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd, void *arg)
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+ void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
(IOCTL32_Command_struct __user *) arg;
@@ -4810,7 +4813,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd, void *arg)
if (err)
return -EFAULT;
- err = hpsa_ioctl(dev, CCISS_PASSTHRU, (void *)p);
+ err = hpsa_ioctl(dev, CCISS_PASSTHRU, p);
if (err)
return err;
err |= copy_in_user(&arg32->error_info, &p->error_info,
@@ -4821,7 +4824,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd, void *arg)
}
static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void *arg)
+ int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -4848,7 +4851,7 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
if (err)
return -EFAULT;
- err = hpsa_ioctl(dev, CCISS_BIG_PASSTHRU, (void *)p);
+ err = hpsa_ioctl(dev, CCISS_BIG_PASSTHRU, p);
if (err)
return err;
err |= copy_in_user(&arg32->error_info, &p->error_info,
@@ -4858,7 +4861,7 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
return err;
}
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -5206,7 +5209,7 @@ static void decrement_passthru_count(struct ctlr_info *h)
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void *arg)
+static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
@@ -5818,7 +5821,7 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode,
#define hpsa_noop(p) hpsa_message(p, 3, 0)
static int hpsa_controller_hard_reset(struct pci_dev *pdev,
- void * __iomem vaddr, u32 use_doorbell)
+ void __iomem *vaddr, u32 use_doorbell)
{
u16 pmcsr;
int pos;
@@ -6056,7 +6059,7 @@ unmap_vaddr:
* the io functions.
* This is for debug only.
*/
-static void print_cfg_table(struct device *dev, struct CfgTable *tb)
+static void print_cfg_table(struct device *dev, struct CfgTable __iomem *tb)
{
#ifdef HPSA_DEBUG
int i;
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 24472ce..80fa9a9 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -164,7 +164,7 @@ struct ctlr_info {
*/
u32 trans_support;
u32 trans_offset;
- struct TransTable_struct *transtable;
+ struct TransTable_struct __iomem *transtable;
unsigned long transMethod;
/* cap concurrent passthrus at some reasonable maximum */
@@ -181,7 +181,7 @@ struct ctlr_info {
u32 *blockFetchTable;
u32 *ioaccel1_blockFetchTable;
u32 *ioaccel2_blockFetchTable;
- u32 *ioaccel2_bft2_regs;
+ u32 __iomem *ioaccel2_bft2_regs;
unsigned char *hba_inquiry_data;
u32 driver_support;
u32 fw_support;
@@ -192,7 +192,7 @@ struct ctlr_info {
u64 last_heartbeat_timestamp;
u32 heartbeat_sample_interval;
atomic_t firmware_flash_in_progress;
- u32 *lockup_detected;
+ u32 __percpu *lockup_detected;
struct delayed_work monitor_ctlr_work;
int remove_in_progress;
u32 fifo_recently_full;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 02/13] hpsa: remove dev_warn prints from RAID-1ADM
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
2014-11-14 23:26 ` [PATCH 01/13] hpsa: Clean up warnings from sparse Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:26 ` [PATCH 03/13] hpsa: fix a couple pci id table mistakes Don Brace
` (11 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Robert Elliott <elliott@hp.com>
RAID-1ADM is unusable with dev_warn called on every command.
Signed-off-by: Robert Elliott <elliott@hp.com>
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Stephen M. Cameron <stephenmcameron@gmail.com>
Reviewed-by: Webb Scales <webbnh@hp.com>
---
drivers/scsi/hpsa.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index f345cc5..e24b304 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3811,11 +3811,6 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
offload_to_mirror =
(offload_to_mirror >= map->layout_map_count - 1)
? 0 : offload_to_mirror + 1;
- /* FIXME: remove after debug/dev */
- BUG_ON(offload_to_mirror >= map->layout_map_count);
- dev_warn(&h->pdev->dev,
- "DEBUG: Using physical disk map index %d from mirror group %d\n",
- map_index, offload_to_mirror);
dev->offload_to_mirror = offload_to_mirror;
/* Avoid direct use of dev->offload_to_mirror within this
* function since multiple threads might simultaneously
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 03/13] hpsa: fix a couple pci id table mistakes
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
2014-11-14 23:26 ` [PATCH 01/13] hpsa: Clean up warnings from sparse Don Brace
2014-11-14 23:26 ` [PATCH 02/13] hpsa: remove dev_warn prints from RAID-1ADM Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:26 ` [PATCH 04/13] hpsa: correct off-by-one sizing of chained SG block Don Brace
` (10 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
Fix a couple of pci id table mistakes:
Subdevice ID 0x3323 missing from product[] table
(another name for HP Smart Storage 1210m)
Bogus 0x1925 subdevice id removed from hpsa_pci_device_id[] (no such thing.)
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Webb Scales <webbnh@hp.com>
---
drivers/scsi/hpsa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index e24b304..ed92e9e 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -104,7 +104,6 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1922},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1923},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1924},
- {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1925},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1926},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1928},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1929},
@@ -150,6 +149,7 @@ static struct board_type products[] = {
{0x3249103C, "Smart Array P812", &SA5_access},
{0x324A103C, "Smart Array P712m", &SA5_access},
{0x324B103C, "Smart Array P711m", &SA5_access},
+ {0x3233103C, "HP StorageWorks 1210m", &SA5_access}, /* alias of 333f */
{0x3350103C, "Smart Array P222", &SA5_access},
{0x3351103C, "Smart Array P420", &SA5_access},
{0x3352103C, "Smart Array P421", &SA5_access},
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 04/13] hpsa: correct off-by-one sizing of chained SG block
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (2 preceding siblings ...)
2014-11-14 23:26 ` [PATCH 03/13] hpsa: fix a couple pci id table mistakes Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:26 ` [PATCH 05/13] hpsa: remove 'action required' phrasing Don Brace
` (9 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Webb Scales <webbnh@hp.com>
Correct the size calculation of the chained SG block
Signed-off-by: Don Brace <don.brace@pmcs.com>
Signed-off-by: Webb Scales <webbnh@hp.com>
Reviewed-by: Stephen M. Cameron <stephenmcameron@gmail.com>
Reviewed-by: Don Brace <don.brace@pmcs.com>
---
drivers/scsi/hpsa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index ed92e9e..f57081c 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6321,11 +6321,11 @@ static void hpsa_find_board_params(struct ctlr_info *h)
h->max_cmd_sg_entries = 31;
if (h->maxsgentries > 512) {
h->max_cmd_sg_entries = 32;
- h->chainsize = h->maxsgentries - h->max_cmd_sg_entries + 1;
+ h->chainsize = h->maxsgentries - h->max_cmd_sg_entries;
h->maxsgentries--; /* save one for chain pointer */
} else {
- h->maxsgentries = 31; /* default to traditional values */
h->chainsize = 0;
+ h->maxsgentries = 31; /* default to traditional values */
}
/* Find out what task management functions are supported and cache */
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 05/13] hpsa: remove 'action required' phrasing
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (3 preceding siblings ...)
2014-11-14 23:26 ` [PATCH 04/13] hpsa: correct off-by-one sizing of chained SG block Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:26 ` [PATCH 06/13] hpsa: fix allocation sizes for CISS_REPORT_LUNs commands Don Brace
` (8 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
In the case of LUN data changing, the driver will
auto rescan and so it's not even true that "action" is
"required".
Remove "action required" phrases from warning messages and
replace with description phrases.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Stephen M. Cameron <stephenmcameron@gmail.com>
Reviewed-by: Joe Handzik <joseph.t.handzik@hp.com>
Reviewed-by: Webb Scales <webb.scales@hp.com>
---
drivers/scsi/hpsa.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index f57081c..538fdea 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -276,12 +276,12 @@ static int check_for_unit_attention(struct ctlr_info *h,
"detected, command retried\n", h->ctlr);
break;
case LUN_FAILED:
- dev_warn(&h->pdev->dev, HPSA "%d: LUN failure "
- "detected, action required\n", h->ctlr);
+ dev_warn(&h->pdev->dev,
+ HPSA "%d: LUN failure detected\n", h->ctlr);
break;
case REPORT_LUNS_CHANGED:
- dev_warn(&h->pdev->dev, HPSA "%d: report LUN data "
- "changed, action required\n", h->ctlr);
+ dev_warn(&h->pdev->dev,
+ HPSA "%d: report LUN data changed\n", h->ctlr);
/*
* Note: this REPORT_LUNS_CHANGED condition only occurs on the external
* target (array) devices.
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 06/13] hpsa: fix allocation sizes for CISS_REPORT_LUNs commands
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (4 preceding siblings ...)
2014-11-14 23:26 ` [PATCH 05/13] hpsa: remove 'action required' phrasing Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:26 ` [PATCH 07/13] hpsa: fix endianness issue with scatter gather elements Don Brace
` (7 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
We were allocating roughly double the amount of memory
we should be due to ReportLUNdata and ExtendedReportLUNdata
containing a non-zero sized array but adding extra memory
to allocate as if the array were zero sized.
Track the logical and physical sizes separately.
Allocate the memory based on the specific data
structure sizes.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Webb Scales <webb.scales@hp.com>
---
drivers/scsi/hpsa.c | 14 +++++++-------
drivers/scsi/hpsa_cmd.h | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 538fdea..a221f7b 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2893,7 +2893,7 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_info *h,
* Returns 0 on success, -1 otherwise.
*/
static int hpsa_gather_lun_info(struct ctlr_info *h,
- int reportlunsize,
+ int reportphyslunsize, int reportloglunsize,
struct ReportLUNdata *physdev, u32 *nphysicals, int *physical_mode,
struct ReportLUNdata *logdev, u32 *nlogicals)
{
@@ -2907,7 +2907,7 @@ static int hpsa_gather_lun_info(struct ctlr_info *h,
*physical_mode = HPSA_REPORT_PHYS_EXTENDED;
physical_entry_size = 24;
}
- if (hpsa_scsi_do_report_phys_luns(h, physdev, reportlunsize,
+ if (hpsa_scsi_do_report_phys_luns(h, physdev, reportphyslunsize,
*physical_mode)) {
dev_err(&h->pdev->dev, "report physical LUNs failed.\n");
return -1;
@@ -2920,7 +2920,7 @@ static int hpsa_gather_lun_info(struct ctlr_info *h,
*nphysicals - HPSA_MAX_PHYS_LUN);
*nphysicals = HPSA_MAX_PHYS_LUN;
}
- if (hpsa_scsi_do_report_log_luns(h, logdev, reportlunsize)) {
+ if (hpsa_scsi_do_report_log_luns(h, logdev, reportloglunsize)) {
dev_err(&h->pdev->dev, "report logical LUNs failed.\n");
return -1;
}
@@ -3013,15 +3013,14 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
u32 ndev_allocated = 0;
struct hpsa_scsi_dev_t **currentsd, *this_device, *tmpdevice;
int ncurrent = 0;
- int reportlunsize = sizeof(*physdev_list) + HPSA_MAX_PHYS_LUN * 24;
int i, n_ext_target_devs, ndevs_to_allocate;
int raid_ctlr_position;
int rescan_hba_mode;
DECLARE_BITMAP(lunzerobits, MAX_EXT_TARGETS);
currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);
- physdev_list = kzalloc(reportlunsize, GFP_KERNEL);
- logdev_list = kzalloc(reportlunsize, GFP_KERNEL);
+ physdev_list = kzalloc(sizeof(*physdev_list), GFP_KERNEL);
+ logdev_list = kzalloc(sizeof(*logdev_list), GFP_KERNEL);
tmpdevice = kzalloc(sizeof(*tmpdevice), GFP_KERNEL);
if (!currentsd || !physdev_list || !logdev_list || !tmpdevice) {
@@ -3041,7 +3040,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
h->hba_mode_enabled = rescan_hba_mode;
- if (hpsa_gather_lun_info(h, reportlunsize,
+ if (hpsa_gather_lun_info(h,
+ sizeof(*physdev_list), sizeof(*logdev_list),
(struct ReportLUNdata *) physdev_list, &nphysicals,
&physical_mode, logdev_list, &nlogicals))
goto out;
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index b5125dc..9b19042f 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -252,7 +252,7 @@ struct ReportExtendedLUNdata {
u8 LUNListLength[4];
u8 extended_response_flag;
u8 reserved[3];
- struct ext_report_lun_entry LUN[HPSA_MAX_LUN];
+ struct ext_report_lun_entry LUN[HPSA_MAX_PHYS_LUN];
};
struct SenseSubsystem_info {
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 07/13] hpsa: fix endianness issue with scatter gather elements
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (5 preceding siblings ...)
2014-11-14 23:26 ` [PATCH 06/13] hpsa: fix allocation sizes for CISS_REPORT_LUNs commands Don Brace
@ 2014-11-14 23:26 ` Don Brace
2014-11-14 23:27 ` [PATCH 08/13] hpsa: get rid of type/attribute/direction bit field where possible Don Brace
` (6 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:26 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
The hardware needs little endian scatter gather addresses and
lengths but we were not bothering to convert from cpu byte
order as we should have been. On Intel, this is all just
a bunch of no-ops macros, but it makes the code endian-clean(er).
Signed-off-by: Don Brace <don.brace@pmcs.com>
Signed-off-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Webb Scales <webbnh@hp.com>
---
drivers/scsi/hpsa.c | 223 +++++++++++++++++++++--------------------------
drivers/scsi/hpsa_cmd.h | 14 +--
2 files changed, 107 insertions(+), 130 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a221f7b..fe1b589 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1502,22 +1502,22 @@ static int hpsa_map_sg_chain_block(struct ctlr_info *h,
{
struct SGDescriptor *chain_sg, *chain_block;
u64 temp64;
+ u32 chain_len;
chain_sg = &c->SG[h->max_cmd_sg_entries - 1];
chain_block = h->cmd_sg_list[c->cmdindex];
- chain_sg->Ext = HPSA_SG_CHAIN;
- chain_sg->Len = sizeof(*chain_sg) *
+ chain_sg->Ext = cpu_to_le32(HPSA_SG_CHAIN);
+ chain_len = sizeof(*chain_sg) *
(c->Header.SGTotal - h->max_cmd_sg_entries);
- temp64 = pci_map_single(h->pdev, chain_block, chain_sg->Len,
+ chain_sg->Len = cpu_to_le32(chain_len);
+ temp64 = pci_map_single(h->pdev, chain_block, chain_len,
PCI_DMA_TODEVICE);
if (dma_mapping_error(&h->pdev->dev, temp64)) {
/* prevent subsequent unmapping */
- chain_sg->Addr.lower = 0;
- chain_sg->Addr.upper = 0;
+ chain_sg->Addr = cpu_to_le64(0);
return -1;
}
- chain_sg->Addr.lower = (u32) (temp64 & 0x0FFFFFFFFULL);
- chain_sg->Addr.upper = (u32) ((temp64 >> 32) & 0x0FFFFFFFFULL);
+ chain_sg->Addr = cpu_to_le64(temp64);
return 0;
}
@@ -1525,15 +1525,13 @@ static void hpsa_unmap_sg_chain_block(struct ctlr_info *h,
struct CommandList *c)
{
struct SGDescriptor *chain_sg;
- union u64bit temp64;
- if (c->Header.SGTotal <= h->max_cmd_sg_entries)
+ if (le16_to_cpu(c->Header.SGTotal) <= h->max_cmd_sg_entries)
return;
chain_sg = &c->SG[h->max_cmd_sg_entries - 1];
- temp64.val32.lower = chain_sg->Addr.lower;
- temp64.val32.upper = chain_sg->Addr.upper;
- pci_unmap_single(h->pdev, temp64.val, chain_sg->Len, PCI_DMA_TODEVICE);
+ pci_unmap_single(h->pdev, le64_to_cpu(chain_sg->Addr),
+ le32_to_cpu(chain_sg->Len), PCI_DMA_TODEVICE);
}
@@ -1734,8 +1732,7 @@ static void complete_scsi_command(struct CommandList *cp)
struct io_accel1_cmd *c = &h->ioaccel_cmd_pool[cp->cmdindex];
cp->Header.SGList = cp->Header.SGTotal = scsi_sg_count(cmd);
cp->Request.CDBLen = c->io_flags & IOACCEL1_IOFLAGS_CDBLEN_MASK;
- cp->Header.Tag.lower = c->Tag.lower;
- cp->Header.Tag.upper = c->Tag.upper;
+ cp->Header.tag = c->tag;
memcpy(cp->Header.LUN.LunAddrBytes, c->CISS_LUN, 8);
memcpy(cp->Request.CDB, c->CDB, cp->Request.CDBLen);
@@ -1936,14 +1933,11 @@ static void hpsa_pci_unmap(struct pci_dev *pdev,
struct CommandList *c, int sg_used, int data_direction)
{
int i;
- union u64bit addr64;
- for (i = 0; i < sg_used; i++) {
- addr64.val32.lower = c->SG[i].Addr.lower;
- addr64.val32.upper = c->SG[i].Addr.upper;
- pci_unmap_single(pdev, (dma_addr_t) addr64.val, c->SG[i].Len,
- data_direction);
- }
+ for (i = 0; i < sg_used; i++)
+ pci_unmap_single(pdev, (dma_addr_t) le64_to_cpu(c->SG[i].Addr),
+ le32_to_cpu(c->SG[i].Len),
+ data_direction);
}
static int hpsa_map_one(struct pci_dev *pdev,
@@ -1956,25 +1950,22 @@ static int hpsa_map_one(struct pci_dev *pdev,
if (buflen == 0 || data_direction == PCI_DMA_NONE) {
cp->Header.SGList = 0;
- cp->Header.SGTotal = 0;
+ cp->Header.SGTotal = cpu_to_le16(0);
return 0;
}
- addr64 = (u64) pci_map_single(pdev, buf, buflen, data_direction);
+ addr64 = pci_map_single(pdev, buf, buflen, data_direction);
if (dma_mapping_error(&pdev->dev, addr64)) {
/* Prevent subsequent unmap of something never mapped */
cp->Header.SGList = 0;
- cp->Header.SGTotal = 0;
+ cp->Header.SGTotal = cpu_to_le16(0);
return -1;
}
- cp->SG[0].Addr.lower =
- (u32) (addr64 & (u64) 0x00000000FFFFFFFF);
- cp->SG[0].Addr.upper =
- (u32) ((addr64 >> 32) & (u64) 0x00000000FFFFFFFF);
- cp->SG[0].Len = buflen;
- cp->SG[0].Ext = HPSA_SG_LAST; /* we are not chaining */
- cp->Header.SGList = (u8) 1; /* no. SGs contig in this cmd */
- cp->Header.SGTotal = (u16) 1; /* total sgs in this cmd list */
+ cp->SG[0].Addr = cpu_to_le64(addr64);
+ cp->SG[0].Len = cpu_to_le32(buflen);
+ cp->SG[0].Ext = cpu_to_le32(HPSA_SG_LAST); /* we are not chaining */
+ cp->Header.SGList = 1; /* no. SGs contig in this cmd */
+ cp->Header.SGTotal = cpu_to_le16(1); /* total sgs in cmd list */
return 0;
}
@@ -2832,8 +2823,8 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_info *h,
if (d == NULL)
return 0; /* no match */
- it_nexus = cpu_to_le32((u32) d->ioaccel_handle);
- scsi_nexus = cpu_to_le32((u32) c2a->scsi_nexus);
+ it_nexus = cpu_to_le32(d->ioaccel_handle);
+ scsi_nexus = cpu_to_le32(c2a->scsi_nexus);
find = c2a->scsi_nexus;
if (h->raid_offload_debug > 0)
@@ -3212,19 +3203,19 @@ static int hpsa_scatter_gather(struct ctlr_info *h,
}
addr64 = (u64) sg_dma_address(sg);
len = sg_dma_len(sg);
- curr_sg->Addr.lower = (u32) (addr64 & 0x0FFFFFFFFULL);
- curr_sg->Addr.upper = (u32) ((addr64 >> 32) & 0x0FFFFFFFFULL);
- curr_sg->Len = len;
- curr_sg->Ext = (i < scsi_sg_count(cmd) - 1) ? 0 : HPSA_SG_LAST;
+ curr_sg->Addr = cpu_to_le64(addr64);
+ curr_sg->Len = cpu_to_le32(len);
+ curr_sg->Ext = cpu_to_le32(0);
curr_sg++;
}
+ (--curr_sg)->Ext = cpu_to_le32(HPSA_SG_LAST);
if (use_sg + chained > h->maxSG)
h->maxSG = use_sg + chained;
if (chained) {
cp->Header.SGList = h->max_cmd_sg_entries;
- cp->Header.SGTotal = (u16) (use_sg + 1);
+ cp->Header.SGTotal = cpu_to_le16(use_sg + 1);
if (hpsa_map_sg_chain_block(h, cp)) {
scsi_dma_unmap(cmd);
return -1;
@@ -3235,7 +3226,7 @@ static int hpsa_scatter_gather(struct ctlr_info *h,
sglist_finished:
cp->Header.SGList = (u8) use_sg; /* no. SGs contig in this cmd */
- cp->Header.SGTotal = (u16) use_sg; /* total sgs in this cmd list */
+ cp->Header.SGTotal = cpu_to_le16(use_sg); /* total sgs in this cmd list */
return 0;
}
@@ -3327,17 +3318,12 @@ static int hpsa_scsi_ioaccel1_queue_command(struct ctlr_info *h,
addr64 = (u64) sg_dma_address(sg);
len = sg_dma_len(sg);
total_len += len;
- curr_sg->Addr.lower = (u32) (addr64 & 0x0FFFFFFFFULL);
- curr_sg->Addr.upper =
- (u32) ((addr64 >> 32) & 0x0FFFFFFFFULL);
- curr_sg->Len = len;
-
- if (i == (scsi_sg_count(cmd) - 1))
- curr_sg->Ext = HPSA_SG_LAST;
- else
- curr_sg->Ext = 0; /* we are not chaining */
+ curr_sg->Addr = cpu_to_le64(addr64);
+ curr_sg->Len = cpu_to_le32(len);
+ curr_sg->Ext = cpu_to_le32(0);
curr_sg++;
}
+ (--curr_sg)->Ext = cpu_to_le32(HPSA_SG_LAST);
switch (cmd->sc_data_direction) {
case DMA_TO_DEVICE:
@@ -3594,7 +3580,7 @@ static int hpsa_scsi_ioaccel2_queue_command(struct ctlr_info *h,
cp->data_len = cpu_to_le32(total_len);
cp->err_ptr = cpu_to_le64(c->busaddr +
offsetof(struct io_accel2_cmd, error_data));
- cp->err_len = cpu_to_le32((u32) sizeof(cp->error_data));
+ cp->err_len = cpu_to_le32(sizeof(cp->error_data));
enqueue_cmd_and_start_io(h, c);
return 0;
@@ -4023,8 +4009,8 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
c->Header.ReplyQueue = 0; /* unused in simple mode */
memcpy(&c->Header.LUN.LunAddrBytes[0], &scsi3addr[0], 8);
- c->Header.Tag.lower = (c->cmdindex << DIRECT_LOOKUP_SHIFT);
- c->Header.Tag.lower |= DIRECT_LOOKUP_BIT;
+ c->Header.tag = cpu_to_le64((c->cmdindex << DIRECT_LOOKUP_SHIFT) |
+ DIRECT_LOOKUP_BIT);
/* Fill in the request block... */
@@ -4326,8 +4312,8 @@ static void hpsa_get_tag(struct ctlr_info *h,
if (c->cmd_type == CMD_IOACCEL1) {
struct io_accel1_cmd *cm1 = (struct io_accel1_cmd *)
&h->ioaccel_cmd_pool[c->cmdindex];
- *tagupper = cm1->Tag.upper;
- *taglower = cm1->Tag.lower;
+ *tagupper = (u32) (cm1->tag >> 32);
+ *taglower = (u32) (cm1->tag & 0x0ffffffffULL);
return;
}
if (c->cmd_type == CMD_IOACCEL2) {
@@ -4338,11 +4324,10 @@ static void hpsa_get_tag(struct ctlr_info *h,
*taglower = cm2->Tag;
return;
}
- *tagupper = c->Header.Tag.upper;
- *taglower = c->Header.Tag.lower;
+ *tagupper = (u32) (c->Header.tag >> 32);
+ *taglower = (u32) (c->Header.tag & 0x0ffffffffULL);
}
-
static int hpsa_send_abort(struct ctlr_info *h, unsigned char *scsi3addr,
struct CommandList *abort, int swizzle)
{
@@ -4429,7 +4414,7 @@ static struct CommandList *hpsa_find_cmd_in_queue_by_tag(struct ctlr_info *h,
spin_lock_irqsave(&h->lock, flags);
list_for_each_entry(c, queue_head, list) {
- if (memcmp(&c->Header.Tag, tag, 8) != 0)
+ if (memcmp(&c->Header.tag, tag, 8) != 0)
continue;
spin_unlock_irqrestore(&h->lock, flags);
return c;
@@ -4711,9 +4696,8 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
INIT_LIST_HEAD(&c->list);
c->busaddr = (u32) cmd_dma_handle;
temp64.val = (u64) err_dma_handle;
- c->ErrDesc.Addr.lower = temp64.val32.lower;
- c->ErrDesc.Addr.upper = temp64.val32.upper;
- c->ErrDesc.Len = sizeof(*c->err_info);
+ c->ErrDesc.Addr = cpu_to_le64(err_dma_handle);
+ c->ErrDesc.Len = cpu_to_le32(sizeof(*c->err_info));
c->h = h;
return c;
@@ -4726,7 +4710,6 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
static struct CommandList *cmd_special_alloc(struct ctlr_info *h)
{
struct CommandList *c;
- union u64bit temp64;
dma_addr_t cmd_dma_handle, err_dma_handle;
c = pci_zalloc_consistent(h->pdev, sizeof(*c), &cmd_dma_handle);
@@ -4747,10 +4730,8 @@ static struct CommandList *cmd_special_alloc(struct ctlr_info *h)
INIT_LIST_HEAD(&c->list);
c->busaddr = (u32) cmd_dma_handle;
- temp64.val = (u64) err_dma_handle;
- c->ErrDesc.Addr.lower = temp64.val32.lower;
- c->ErrDesc.Addr.upper = temp64.val32.upper;
- c->ErrDesc.Len = sizeof(*c->err_info);
+ c->ErrDesc.Addr = cpu_to_le64(err_dma_handle);
+ c->ErrDesc.Len = cpu_to_le32(sizeof(*c->err_info));
c->h = h;
return c;
@@ -4770,12 +4751,9 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)
static void cmd_special_free(struct ctlr_info *h, struct CommandList *c)
{
- union u64bit temp64;
-
- temp64.val32.lower = c->ErrDesc.Addr.lower;
- temp64.val32.upper = c->ErrDesc.Addr.upper;
pci_free_consistent(h->pdev, sizeof(*c->err_info),
- c->err_info, (dma_addr_t) temp64.val);
+ c->err_info,
+ (dma_addr_t) le64_to_cpu(c->ErrDesc.Addr));
pci_free_consistent(h->pdev, sizeof(*c),
c, (dma_addr_t) (c->busaddr & DIRECT_LOOKUP_MASK));
}
@@ -4930,7 +4908,7 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
IOCTL_Command_struct iocommand;
struct CommandList *c;
char *buff = NULL;
- union u64bit temp64;
+ u64 temp64;
int rc = 0;
if (!argp)
@@ -4969,14 +4947,14 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
c->Header.ReplyQueue = 0; /* unused in simple mode */
if (iocommand.buf_size > 0) { /* buffer to fill */
c->Header.SGList = 1;
- c->Header.SGTotal = 1;
+ c->Header.SGTotal = cpu_to_le16(1);
} else { /* no buffers to fill */
c->Header.SGList = 0;
- c->Header.SGTotal = 0;
+ c->Header.SGTotal = cpu_to_le16(0);
}
memcpy(&c->Header.LUN, &iocommand.LUN_info, sizeof(c->Header.LUN));
/* use the kernel address the cmd block for tag */
- c->Header.Tag.lower = c->busaddr;
+ c->Header.tag = c->busaddr;
/* Fill in Request block */
memcpy(&c->Request, &iocommand.Request,
@@ -4984,19 +4962,17 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
/* Fill in the scatter gather information */
if (iocommand.buf_size > 0) {
- temp64.val = pci_map_single(h->pdev, buff,
+ temp64 = pci_map_single(h->pdev, buff,
iocommand.buf_size, PCI_DMA_BIDIRECTIONAL);
- if (dma_mapping_error(&h->pdev->dev, temp64.val)) {
- c->SG[0].Addr.lower = 0;
- c->SG[0].Addr.upper = 0;
- c->SG[0].Len = 0;
+ if (dma_mapping_error(&h->pdev->dev, (dma_addr_t) temp64)) {
+ c->SG[0].Addr = cpu_to_le64(0);
+ c->SG[0].Len = cpu_to_le32(0);
rc = -ENOMEM;
goto out;
}
- c->SG[0].Addr.lower = temp64.val32.lower;
- c->SG[0].Addr.upper = temp64.val32.upper;
- c->SG[0].Len = iocommand.buf_size;
- c->SG[0].Ext = HPSA_SG_LAST; /* we are not chaining*/
+ c->SG[0].Addr = cpu_to_le64(temp64);
+ c->SG[0].Len = cpu_to_le32(iocommand.buf_size);
+ c->SG[0].Ext = cpu_to_le32(HPSA_SG_LAST); /* not chaining */
}
hpsa_scsi_do_simple_cmd_core_if_no_lockup(h, c);
if (iocommand.buf_size > 0)
@@ -5031,7 +5007,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp)
struct CommandList *c;
unsigned char **buff = NULL;
int *buff_size = NULL;
- union u64bit temp64;
+ u64 temp64;
BYTE sg_used = 0;
int status = 0;
int i;
@@ -5105,29 +5081,30 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp)
}
c->cmd_type = CMD_IOCTL_PEND;
c->Header.ReplyQueue = 0;
- c->Header.SGList = c->Header.SGTotal = sg_used;
+ c->Header.SGList = (u8) sg_used;
+ c->Header.SGTotal = cpu_to_le16(sg_used);
memcpy(&c->Header.LUN, &ioc->LUN_info, sizeof(c->Header.LUN));
- c->Header.Tag.lower = c->busaddr;
+ c->Header.tag = c->busaddr;
memcpy(&c->Request, &ioc->Request, sizeof(c->Request));
if (ioc->buf_size > 0) {
int i;
for (i = 0; i < sg_used; i++) {
- temp64.val = pci_map_single(h->pdev, buff[i],
+ temp64 = pci_map_single(h->pdev, buff[i],
buff_size[i], PCI_DMA_BIDIRECTIONAL);
- if (dma_mapping_error(&h->pdev->dev, temp64.val)) {
- c->SG[i].Addr.lower = 0;
- c->SG[i].Addr.upper = 0;
- c->SG[i].Len = 0;
+ if (dma_mapping_error(&h->pdev->dev,
+ (dma_addr_t) temp64)) {
+ c->SG[i].Addr = cpu_to_le64(0);
+ c->SG[i].Len = cpu_to_le32(0);
hpsa_pci_unmap(h->pdev, c, i,
PCI_DMA_BIDIRECTIONAL);
status = -ENOMEM;
goto cleanup0;
}
- c->SG[i].Addr.lower = temp64.val32.lower;
- c->SG[i].Addr.upper = temp64.val32.upper;
- c->SG[i].Len = buff_size[i];
- c->SG[i].Ext = i < sg_used - 1 ? 0 : HPSA_SG_LAST;
+ c->SG[i].Addr = cpu_to_le64(temp64);
+ c->SG[i].Len = cpu_to_le32(buff_size[i]);
+ c->SG[i].Ext = cpu_to_le32(0);
}
+ c->SG[--i].Ext = cpu_to_le32(HPSA_SG_LAST);
}
hpsa_scsi_do_simple_cmd_core_if_no_lockup(h, c);
if (sg_used)
@@ -5266,17 +5243,18 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
{
int pci_dir = XFER_NONE;
struct CommandList *a; /* for commands to be aborted */
+ u32 tupper, tlower;
c->cmd_type = CMD_IOCTL_PEND;
c->Header.ReplyQueue = 0;
if (buff != NULL && size > 0) {
c->Header.SGList = 1;
- c->Header.SGTotal = 1;
+ c->Header.SGTotal = cpu_to_le16(1);
} else {
c->Header.SGList = 0;
- c->Header.SGTotal = 0;
+ c->Header.SGTotal = cpu_to_le16(0);
}
- c->Header.Tag.lower = c->busaddr;
+ c->Header.tag = c->busaddr;
memcpy(c->Header.LUN.LunAddrBytes, scsi3addr, 8);
c->Request.Type.Type = cmd_type;
@@ -5374,9 +5352,10 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case HPSA_ABORT_MSG:
a = buff; /* point to command to be aborted */
- dev_dbg(&h->pdev->dev, "Abort Tag:0x%08x:%08x using request Tag:0x%08x:%08x\n",
- a->Header.Tag.upper, a->Header.Tag.lower,
- c->Header.Tag.upper, c->Header.Tag.lower);
+ dev_dbg(&h->pdev->dev, "Abort Tag:0x%016llx using request Tag:0x%016llx",
+ a->Header.tag, c->Header.tag);
+ tlower = (u32) (a->Header.tag >> 32);
+ tupper = (u32) (a->Header.tag & 0x0ffffffffULL);
c->Request.CDBLen = 16;
c->Request.Type.Type = TYPE_MSG;
c->Request.Type.Attribute = ATTR_SIMPLE;
@@ -5387,14 +5366,14 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
c->Request.CDB[2] = 0x00; /* reserved */
c->Request.CDB[3] = 0x00; /* reserved */
/* Tag to abort goes in CDB[4]-CDB[11] */
- c->Request.CDB[4] = a->Header.Tag.lower & 0xFF;
- c->Request.CDB[5] = (a->Header.Tag.lower >> 8) & 0xFF;
- c->Request.CDB[6] = (a->Header.Tag.lower >> 16) & 0xFF;
- c->Request.CDB[7] = (a->Header.Tag.lower >> 24) & 0xFF;
- c->Request.CDB[8] = a->Header.Tag.upper & 0xFF;
- c->Request.CDB[9] = (a->Header.Tag.upper >> 8) & 0xFF;
- c->Request.CDB[10] = (a->Header.Tag.upper >> 16) & 0xFF;
- c->Request.CDB[11] = (a->Header.Tag.upper >> 24) & 0xFF;
+ c->Request.CDB[4] = tlower & 0xFF;
+ c->Request.CDB[5] = (tlower >> 8) & 0xFF;
+ c->Request.CDB[6] = (tlower >> 16) & 0xFF;
+ c->Request.CDB[7] = (tlower >> 24) & 0xFF;
+ c->Request.CDB[8] = tupper & 0xFF;
+ c->Request.CDB[9] = (tupper >> 8) & 0xFF;
+ c->Request.CDB[10] = (tupper >> 16) & 0xFF;
+ c->Request.CDB[11] = (tupper >> 24) & 0xFF;
c->Request.CDB[12] = 0x00; /* reserved */
c->Request.CDB[13] = 0x00; /* reserved */
c->Request.CDB[14] = 0x00; /* reserved */
@@ -5763,9 +5742,8 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode,
cmd->CommandHeader.ReplyQueue = 0;
cmd->CommandHeader.SGList = 0;
- cmd->CommandHeader.SGTotal = 0;
- cmd->CommandHeader.Tag.lower = paddr32;
- cmd->CommandHeader.Tag.upper = 0;
+ cmd->CommandHeader.SGTotal = cpu_to_le16(0);
+ cmd->CommandHeader.tag = paddr32;
memset(&cmd->CommandHeader.LUN.LunAddrBytes, 0, 8);
cmd->Request.CDBLen = 16;
@@ -5776,9 +5754,9 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode,
cmd->Request.CDB[0] = opcode;
cmd->Request.CDB[1] = type;
memset(&cmd->Request.CDB[2], 0, 14); /* rest of the CDB is reserved */
- cmd->ErrorDescriptor.Addr.lower = paddr32 + sizeof(*cmd);
- cmd->ErrorDescriptor.Addr.upper = 0;
- cmd->ErrorDescriptor.Len = sizeof(struct ErrorInfo);
+ cmd->ErrorDescriptor.Addr =
+ cpu_to_le64((paddr32 + sizeof(*cmd)));
+ cmd->ErrorDescriptor.Len = cpu_to_le32(sizeof(struct ErrorInfo));
writel(paddr32, vaddr + SA5_REQUEST_PORT_OFFSET);
@@ -7429,13 +7407,12 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support)
cp->host_context_flags = IOACCEL1_HCFLAGS_CISS_FORMAT;
cp->timeout_sec = 0;
cp->ReplyQueue = 0;
- cp->Tag.lower = (i << DIRECT_LOOKUP_SHIFT) |
- DIRECT_LOOKUP_BIT;
- cp->Tag.upper = 0;
- cp->host_addr.lower =
- (u32) (h->ioaccel_cmd_pool_dhandle +
+ cp->tag =
+ cpu_to_le64((i << DIRECT_LOOKUP_SHIFT) |
+ DIRECT_LOOKUP_BIT);
+ cp->host_addr =
+ cpu_to_le64(h->ioaccel_cmd_pool_dhandle +
(i * sizeof(struct io_accel1_cmd)));
- cp->host_addr.upper = 0;
}
} else if (trans_support & CFGTBL_Trans_io_accel2) {
u64 cfg_offset, cfg_base_addr_index;
@@ -7709,7 +7686,7 @@ static void __attribute__((unused)) verify_offsets(void)
VERIFY_OFFSET(timeout_sec, 0x62);
VERIFY_OFFSET(ReplyQueue, 0x64);
VERIFY_OFFSET(reserved9, 0x65);
- VERIFY_OFFSET(Tag, 0x68);
+ VERIFY_OFFSET(tag, 0x68);
VERIFY_OFFSET(host_addr, 0x70);
VERIFY_OFFSET(CISS_LUN, 0x78);
VERIFY_OFFSET(SG, 0x78 + 8);
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index 9b19042f..575eda8 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -314,7 +314,7 @@ struct CommandListHeader {
u8 ReplyQueue;
u8 SGList;
u16 SGTotal;
- struct vals32 Tag;
+ u64 tag;
union LUNAddr LUN;
};
@@ -330,12 +330,12 @@ struct RequestBlock {
};
struct ErrDescriptor {
- struct vals32 Addr;
+ u64 Addr;
u32 Len;
};
struct SGDescriptor {
- struct vals32 Addr;
+ u64 Addr;
u32 Len;
u32 Ext;
};
@@ -434,8 +434,8 @@ struct io_accel1_cmd {
u16 timeout_sec; /* 0x62 - 0x63 */
u8 ReplyQueue; /* 0x64 */
u8 reserved9[3]; /* 0x65 - 0x67 */
- struct vals32 Tag; /* 0x68 - 0x6F */
- struct vals32 host_addr; /* 0x70 - 0x77 */
+ u64 tag; /* 0x68 - 0x6F */
+ u64 host_addr; /* 0x70 - 0x77 */
u8 CISS_LUN[8]; /* 0x78 - 0x7F */
struct SGDescriptor SG[IOACCEL1_MAXSGENTRIES];
} __aligned(IOACCEL1_COMMANDLIST_ALIGNMENT);
@@ -555,8 +555,8 @@ struct hpsa_tmf_struct {
u8 reserved1; /* byte 3 Reserved */
u32 it_nexus; /* SCSI I-T Nexus */
u8 lun_id[8]; /* LUN ID for TMF request */
- struct vals32 Tag; /* cciss tag associated w/ request */
- struct vals32 abort_tag;/* cciss tag of SCSI cmd or task to abort */
+ u64 tag; /* cciss tag associated w/ request */
+ u64 abort_tag; /* cciss tag of SCSI cmd or task to abort */
u64 error_ptr; /* Error Pointer */
u32 error_len; /* Error Length */
};
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 08/13] hpsa: get rid of type/attribute/direction bit field where possible
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (6 preceding siblings ...)
2014-11-14 23:26 ` [PATCH 07/13] hpsa: fix endianness issue with scatter gather elements Don Brace
@ 2014-11-14 23:27 ` Don Brace
2014-11-14 23:27 ` [PATCH 09/13] hpsa: use atomics for commands_outstanding Don Brace
` (5 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:27 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
Using bit fields for hardware command fields isn't portable and
relies on assumptions about how the compiler lays out the bits.
We can fix this in the driver's internal command structure, but the
ioctl interface we can't change because it is part of the
userland ABI.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Webb Scales <webb.scales@hp.com>
---
drivers/scsi/hpsa.c | 58 ++++++++++++++++++++++++-----------------------
drivers/scsi/hpsa_cmd.h | 18 +++++++++++----
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index fe1b589..95c34a5 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4019,17 +4019,18 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
BUG_ON(cmd->cmd_len > sizeof(c->Request.CDB));
c->Request.CDBLen = cmd->cmd_len;
memcpy(c->Request.CDB, cmd->cmnd, cmd->cmd_len);
- c->Request.Type.Type = TYPE_CMD;
- c->Request.Type.Attribute = ATTR_SIMPLE;
switch (cmd->sc_data_direction) {
case DMA_TO_DEVICE:
- c->Request.Type.Direction = XFER_WRITE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_WRITE);
break;
case DMA_FROM_DEVICE:
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_READ);
break;
case DMA_NONE:
- c->Request.Type.Direction = XFER_NONE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_NONE);
break;
case DMA_BIDIRECTIONAL:
/* This can happen if a buggy application does a scsi passthru
@@ -4037,7 +4038,8 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
* ../scsi/scsi_ioctl.c:scsi_ioctl_send_command() )
*/
- c->Request.Type.Direction = XFER_RSVD;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_RSVD);
/* This is technically wrong, and hpsa controllers should
* reject it with CMD_INVALID, which is the most correct
* response, but non-fibre backends appear to let it
@@ -5257,7 +5259,6 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
c->Header.tag = c->busaddr;
memcpy(c->Header.LUN.LunAddrBytes, scsi3addr, 8);
- c->Request.Type.Type = cmd_type;
if (cmd_type == TYPE_CMD) {
switch (cmd) {
case HPSA_INQUIRY:
@@ -5267,8 +5268,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
c->Request.CDB[2] = (page_code & 0xff);
}
c->Request.CDBLen = 6;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = HPSA_INQUIRY;
c->Request.CDB[4] = size & 0xFF;
@@ -5279,8 +5280,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
mode = 00 target = 0. Nothing to write.
*/
c->Request.CDBLen = 12;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = cmd;
c->Request.CDB[6] = (size >> 24) & 0xFF; /* MSB */
@@ -5290,8 +5291,9 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case HPSA_CACHE_FLUSH:
c->Request.CDBLen = 12;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_WRITE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type,
+ ATTR_SIMPLE, XFER_WRITE);
c->Request.Timeout = 0;
c->Request.CDB[0] = BMIC_WRITE;
c->Request.CDB[6] = BMIC_CACHE_FLUSH;
@@ -5300,14 +5302,14 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case TEST_UNIT_READY:
c->Request.CDBLen = 6;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_NONE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_NONE);
c->Request.Timeout = 0;
break;
case HPSA_GET_RAID_MAP:
c->Request.CDBLen = 12;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = HPSA_CISS_READ;
c->Request.CDB[1] = cmd;
@@ -5318,8 +5320,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case BMIC_SENSE_CONTROLLER_PARAMETERS:
c->Request.CDBLen = 10;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_READ;
+ 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[6] = BMIC_SENSE_CONTROLLER_PARAMETERS;
@@ -5336,9 +5338,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
case HPSA_DEVICE_RESET_MSG:
c->Request.CDBLen = 16;
- c->Request.Type.Type = 1; /* It is a MSG not a CMD */
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_NONE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_NONE);
c->Request.Timeout = 0; /* Don't time out */
memset(&c->Request.CDB[0], 0, sizeof(c->Request.CDB));
c->Request.CDB[0] = cmd;
@@ -5357,9 +5358,9 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
tlower = (u32) (a->Header.tag >> 32);
tupper = (u32) (a->Header.tag & 0x0ffffffffULL);
c->Request.CDBLen = 16;
- c->Request.Type.Type = TYPE_MSG;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_WRITE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type,
+ ATTR_SIMPLE, XFER_WRITE);
c->Request.Timeout = 0; /* Don't time out */
c->Request.CDB[0] = HPSA_TASK_MANAGEMENT;
c->Request.CDB[1] = HPSA_TMF_ABORT_TASK;
@@ -5389,7 +5390,7 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
BUG();
}
- switch (c->Request.Type.Direction) {
+ switch (GET_DIR(c->Request.type_attr_dir)) {
case XFER_READ:
pci_dir = PCI_DMA_FROMDEVICE;
break;
@@ -5747,9 +5748,8 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode,
memset(&cmd->CommandHeader.LUN.LunAddrBytes, 0, 8);
cmd->Request.CDBLen = 16;
- cmd->Request.Type.Type = TYPE_MSG;
- cmd->Request.Type.Attribute = ATTR_HEADOFQUEUE;
- cmd->Request.Type.Direction = XFER_NONE;
+ cmd->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_MSG, ATTR_HEADOFQUEUE, XFER_NONE);
cmd->Request.Timeout = 0; /* Don't time out */
cmd->Request.CDB[0] = opcode;
cmd->Request.CDB[1] = type;
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index 575eda8..cb988c4 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -320,11 +320,19 @@ struct CommandListHeader {
struct RequestBlock {
u8 CDBLen;
- struct {
- u8 Type:3;
- u8 Attribute:3;
- u8 Direction:2;
- } Type;
+ /*
+ * type_attr_dir:
+ * type: low 3 bits
+ * attr: middle 3 bits
+ * dir: high 2 bits
+ */
+ u8 type_attr_dir;
+#define TYPE_ATTR_DIR(t, a, d) ((((d) & 0x03) << 6) |\
+ (((a) & 0x07) << 3) |\
+ ((t) & 0x07))
+#define GET_TYPE(tad) ((tad) & 0x07)
+#define GET_ATTR(tad) (((tad) >> 3) & 0x07)
+#define GET_DIR(tad) (((tad) >> 6) & 0x03)
u16 Timeout;
u8 CDB[16];
};
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 09/13] hpsa: use atomics for commands_outstanding
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (7 preceding siblings ...)
2014-11-14 23:27 ` [PATCH 08/13] hpsa: get rid of type/attribute/direction bit field where possible Don Brace
@ 2014-11-14 23:27 ` Don Brace
2014-11-14 23:27 ` [PATCH 10/13] hpsa: do not be so noisy about check conditions Don Brace
` (4 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:27 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
Use atomics for commands_outstanding instead of protecting with spin locks.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Signed-off-by: Stephen M. Cameron <stephenmcameron@gmail.com>
Reviewed-by: Joe Handzik <joseph.t.handzik@hp.com>
---
drivers/scsi/hpsa.c | 26 +++++++++-----------------
drivers/scsi/hpsa.h | 27 +++++++--------------------
2 files changed, 16 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 95c34a5..c9554e6 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -394,7 +394,8 @@ static ssize_t host_show_commands_outstanding(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ctlr_info *h = shost_to_hba(shost);
- return snprintf(buf, 20, "%d\n", h->commands_outstanding);
+ return snprintf(buf, 20, "%d\n",
+ atomic_read(&h->commands_outstanding));
}
static ssize_t host_show_transport_mode(struct device *dev,
@@ -700,7 +701,6 @@ static inline u32 next_command(struct ctlr_info *h, u8 q)
{
u32 a;
struct reply_queue_buffer *rq = &h->reply_queue[q];
- unsigned long flags;
if (h->transMethod & CFGTBL_Trans_io_accel1)
return h->access.command_completed(h, q);
@@ -711,9 +711,7 @@ static inline u32 next_command(struct ctlr_info *h, u8 q)
if ((rq->head[rq->current_entry] & 1) == rq->wraparound) {
a = rq->head[rq->current_entry];
rq->current_entry++;
- spin_lock_irqsave(&h->lock, flags);
- h->commands_outstanding--;
- spin_unlock_irqrestore(&h->lock, flags);
+ atomic_dec(&h->commands_outstanding);
} else {
a = FIFO_EMPTY;
}
@@ -5445,15 +5443,9 @@ static void start_io(struct ctlr_info *h, unsigned long *flags)
/* Put job onto the completed Q */
addQ(&h->cmpQ, c);
-
- /* Must increment commands_outstanding before unlocking
- * and submitting to avoid race checking for fifo full
- * condition.
- */
- h->commands_outstanding++;
-
- /* Tell the controller execute command */
+ atomic_inc(&h->commands_outstanding);
spin_unlock_irqrestore(&h->lock, *flags);
+ /* Tell the controller execute command */
h->access.submit_command(h, c);
spin_lock_irqsave(&h->lock, *flags);
}
@@ -5499,6 +5491,7 @@ static inline void finish_cmd(struct CommandList *c)
unsigned long flags;
int io_may_be_stalled = 0;
struct ctlr_info *h = c->h;
+ int count;
spin_lock_irqsave(&h->lock, flags);
removeQ(c);
@@ -5519,11 +5512,10 @@ static inline void finish_cmd(struct CommandList *c)
* want to get in a cycle where we call start_io every time
* through here.
*/
- if (unlikely(h->fifo_recently_full) &&
- h->commands_outstanding < 5)
- io_may_be_stalled = 1;
-
+ count = atomic_read(&h->commands_outstanding);
spin_unlock_irqrestore(&h->lock, flags);
+ if (unlikely(h->fifo_recently_full) && count < 5)
+ io_may_be_stalled = 1;
dial_up_lockup_detection_on_fw_flash_complete(c->h, c);
if (likely(c->cmd_type == CMD_IOACCEL1 || c->cmd_type == CMD_SCSI
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 80fa9a9..8e06d9e 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -118,7 +118,7 @@ struct ctlr_info {
struct CfgTable __iomem *cfgtable;
int interrupts_enabled;
int max_commands;
- int commands_outstanding;
+ atomic_t commands_outstanding;
# define PERF_MODE_INT 0
# define DOORBELL_INT 1
# define SIMPLE_MODE_INT 2
@@ -395,7 +395,7 @@ static void SA5_performant_intr_mask(struct ctlr_info *h, unsigned long val)
static unsigned long SA5_performant_completed(struct ctlr_info *h, u8 q)
{
struct reply_queue_buffer *rq = &h->reply_queue[q];
- unsigned long flags, register_value = FIFO_EMPTY;
+ unsigned long register_value = FIFO_EMPTY;
/* msi auto clears the interrupt pending bit. */
if (!(h->msi_vector || h->msix_vector)) {
@@ -413,9 +413,7 @@ static unsigned long SA5_performant_completed(struct ctlr_info *h, u8 q)
if ((rq->head[rq->current_entry] & 1) == rq->wraparound) {
register_value = rq->head[rq->current_entry];
rq->current_entry++;
- spin_lock_irqsave(&h->lock, flags);
- h->commands_outstanding--;
- spin_unlock_irqrestore(&h->lock, flags);
+ atomic_dec(&h->commands_outstanding);
} else {
register_value = FIFO_EMPTY;
}
@@ -433,11 +431,7 @@ static unsigned long SA5_performant_completed(struct ctlr_info *h, u8 q)
*/
static unsigned long SA5_fifo_full(struct ctlr_info *h)
{
- if (h->commands_outstanding >= h->max_commands)
- return 1;
- else
- return 0;
-
+ return atomic_read(&h->commands_outstanding) >= h->max_commands;
}
/*
* returns value read from hardware.
@@ -448,13 +442,9 @@ static unsigned long SA5_completed(struct ctlr_info *h,
{
unsigned long register_value
= readl(h->vaddr + SA5_REPLY_PORT_OFFSET);
- unsigned long flags;
- if (register_value != FIFO_EMPTY) {
- spin_lock_irqsave(&h->lock, flags);
- h->commands_outstanding--;
- spin_unlock_irqrestore(&h->lock, flags);
- }
+ if (register_value != FIFO_EMPTY)
+ atomic_dec(&h->commands_outstanding);
#ifdef HPSA_DEBUG
if (register_value != FIFO_EMPTY)
@@ -510,7 +500,6 @@ static unsigned long SA5_ioaccel_mode1_completed(struct ctlr_info *h, u8 q)
{
u64 register_value;
struct reply_queue_buffer *rq = &h->reply_queue[q];
- unsigned long flags;
BUG_ON(q >= h->nreply_queues);
@@ -528,9 +517,7 @@ static unsigned long SA5_ioaccel_mode1_completed(struct ctlr_info *h, u8 q)
wmb();
writel((q << 24) | rq->current_entry, h->vaddr +
IOACCEL_MODE1_CONSUMER_INDEX);
- spin_lock_irqsave(&h->lock, flags);
- h->commands_outstanding--;
- spin_unlock_irqrestore(&h->lock, flags);
+ atomic_dec(&h->commands_outstanding);
}
return (unsigned long) register_value;
}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 10/13] hpsa: do not be so noisy about check conditions
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (8 preceding siblings ...)
2014-11-14 23:27 ` [PATCH 09/13] hpsa: use atomics for commands_outstanding Don Brace
@ 2014-11-14 23:27 ` Don Brace
2014-11-14 23:27 ` [PATCH 11/13] hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation Don Brace
` (3 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:27 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
We were printing a lot of useless information before ultimately
just passing things up to the SCSI mid layer. Just let the
midlayer handle it without LLD chatter.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Signed-off-by: Stephen M. Cameron <stephenmcameron@gmail.com>
Reviewed-by: Joe Handzik <joseph.t.handzik@hp.com>
Reviewed-by: Scott Teel <scott.teel@hp.com>
---
drivers/scsi/hpsa.c | 59 ---------------------------------------------------
1 file changed, 59 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c9554e6..58f6847 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1760,72 +1760,13 @@ static void complete_scsi_command(struct CommandList *cp)
/* Get addition sense code qualifier */
ascq = ei->SenseInfo[13];
}
-
if (ei->ScsiStatus == SAM_STAT_CHECK_CONDITION) {
- if (check_for_unit_attention(h, cp))
- break;
- if (sense_key == ILLEGAL_REQUEST) {
- /*
- * SCSI REPORT_LUNS is commonly unsupported on
- * Smart Array. Suppress noisy complaint.
- */
- if (cp->Request.CDB[0] == REPORT_LUNS)
- break;
-
- /* If ASC/ASCQ indicate Logical Unit
- * Not Supported condition,
- */
- if ((asc == 0x25) && (ascq == 0x0)) {
- dev_warn(&h->pdev->dev, "cp %p "
- "has check condition\n", cp);
- break;
- }
- }
-
- if (sense_key == NOT_READY) {
- /* If Sense is Not Ready, Logical Unit
- * Not ready, Manual Intervention
- * required
- */
- if ((asc == 0x04) && (ascq == 0x03)) {
- dev_warn(&h->pdev->dev, "cp %p "
- "has check condition: unit "
- "not ready, manual "
- "intervention required\n", cp);
- break;
- }
- }
if (sense_key == ABORTED_COMMAND) {
- /* Aborted command is retryable */
- dev_warn(&h->pdev->dev, "cp %p "
- "has check condition: aborted command: "
- "ASC: 0x%x, ASCQ: 0x%x\n",
- cp, asc, ascq);
cmd->result |= DID_SOFT_ERROR << 16;
break;
}
- /* Must be some other type of check condition */
- dev_dbg(&h->pdev->dev, "cp %p has check condition: "
- "unknown type: "
- "Sense: 0x%x, ASC: 0x%x, ASCQ: 0x%x, "
- "Returning result: 0x%x, "
- "cmd=[%02x %02x %02x %02x %02x "
- "%02x %02x %02x %02x %02x %02x "
- "%02x %02x %02x %02x %02x]\n",
- cp, sense_key, asc, ascq,
- cmd->result,
- cmd->cmnd[0], cmd->cmnd[1],
- cmd->cmnd[2], cmd->cmnd[3],
- cmd->cmnd[4], cmd->cmnd[5],
- cmd->cmnd[6], cmd->cmnd[7],
- cmd->cmnd[8], cmd->cmnd[9],
- cmd->cmnd[10], cmd->cmnd[11],
- cmd->cmnd[12], cmd->cmnd[13],
- cmd->cmnd[14], cmd->cmnd[15]);
break;
}
-
-
/* Problem was not a check condition
* Pass it up to the upper layers...
*/
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 11/13] hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (9 preceding siblings ...)
2014-11-14 23:27 ` [PATCH 10/13] hpsa: do not be so noisy about check conditions Don Brace
@ 2014-11-14 23:27 ` Don Brace
2014-11-14 23:27 ` [PATCH 12/13] hpsa: always call pci_set_master after pci_enable_device Don Brace
` (2 subsequent siblings)
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:27 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Nicholas Bellinger <nab@linux-iscsi.org>
There isn't anything in hpsa that requires the host lock to be held
during queuecommand.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Reviewed-by: Stephen M. Cameron <stephenmcameron@gmail.com>
---
drivers/scsi/hpsa.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 58f6847..761612d 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3881,8 +3881,11 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
dev->scsi3addr);
}
-static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
- void (*done)(struct scsi_cmnd *))
+/*
+ * Running in struct Scsi_Host->host_lock less mode using LLD internal
+ * struct ctlr_info *h->lock w/ spin_lock_irqsave() protection.
+ */
+static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
{
struct ctlr_info *h;
struct hpsa_scsi_dev_t *dev;
@@ -3895,14 +3898,14 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
dev = cmd->device->hostdata;
if (!dev) {
cmd->result = DID_NO_CONNECT << 16;
- done(cmd);
+ cmd->scsi_done(cmd);
return 0;
}
memcpy(scsi3addr, dev->scsi3addr, sizeof(scsi3addr));
if (unlikely(lockup_detected(h))) {
cmd->result = DID_ERROR << 16;
- done(cmd);
+ cmd->scsi_done(cmd);
return 0;
}
c = cmd_alloc(h);
@@ -3912,9 +3915,6 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
}
/* Fill in the command list header */
-
- cmd->scsi_done = done; /* save this for use by completion code */
-
/* save c in case we have to abort it */
cmd->host_scribble = (unsigned char *) c;
@@ -4005,8 +4005,6 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
return 0;
}
-static DEF_SCSI_QCMD(hpsa_scsi_queue_command)
-
static int do_not_scan_if_controller_locked_up(struct ctlr_info *h)
{
unsigned long flags;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 12/13] hpsa: always call pci_set_master after pci_enable_device
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (10 preceding siblings ...)
2014-11-14 23:27 ` [PATCH 11/13] hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation Don Brace
@ 2014-11-14 23:27 ` Don Brace
2014-11-14 23:27 ` [PATCH 13/13] hpsa: remove spin lock around command allocation Don Brace
2014-11-20 15:55 ` [PATCH 00/13] hpsa update Christoph Hellwig
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:27 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Robert Elliott <elliott@hp.com>
If the kernel is booted with the reset_device parameter, which
is done for kdump, then the driver needs to call pci_set_master
after pci_enable_device to reenable bus mastering (since
the preceding pci_disable_device call disables bus mastering).
Also, place that after pci_request_regions both in the
kdump code and the normal pci_init code.
Remove the comment summarizing what pci_set_master
does, with the incomplete commentary on the impact of
pci_disable_device.
Signed-off-by: Robert Elliott <elliott@hp.com>
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Don Brace <don.brace@pmcs.com>
---
drivers/scsi/hpsa.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 761612d..72daca9 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6363,15 +6363,15 @@ static int hpsa_pci_init(struct ctlr_info *h)
return err;
}
- /* Enable bus mastering (pci_disable_device may disable this) */
- pci_set_master(h->pdev);
-
err = pci_request_regions(h->pdev, HPSA);
if (err) {
dev_err(&h->pdev->dev,
"cannot obtain PCI resources, aborting\n");
return err;
}
+
+ pci_set_master(h->pdev);
+
hpsa_interrupt_mode(h);
err = hpsa_pci_find_memory_BAR(h->pdev, &h->paddr);
if (err)
@@ -6451,7 +6451,9 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev)
dev_warn(&pdev->dev, "failed to enable device.\n");
return -ENODEV;
}
+
pci_set_master(pdev);
+
/* Reset the controller with a PCI power-cycle or via doorbell */
rc = hpsa_kdump_hard_reset_controller(pdev);
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 13/13] hpsa: remove spin lock around command allocation
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (11 preceding siblings ...)
2014-11-14 23:27 ` [PATCH 12/13] hpsa: always call pci_set_master after pci_enable_device Don Brace
@ 2014-11-14 23:27 ` Don Brace
2014-11-20 15:55 ` [PATCH 00/13] hpsa update Christoph Hellwig
13 siblings, 0 replies; 16+ messages in thread
From: Don Brace @ 2014-11-14 23:27 UTC (permalink / raw)
To: hch, webb.scales, james.bottomley, brace; +Cc: linux-scsi
From: Stephen M. Cameron <stephenmcameron@gmail.com>
It is already using atomic test_and_set_bit to do the
allocation.
There is some microscopic chance of starvation, but it is
so microscopic that it should never happen in reality.
Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Webb Scales <webbnh@hp.com>
---
drivers/scsi/hpsa.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 72daca9..8a3d0de 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4607,19 +4607,32 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
int i;
union u64bit temp64;
dma_addr_t cmd_dma_handle, err_dma_handle;
- unsigned long flags;
+ int loopcount;
+
+ /* There is some *extremely* small but non-zero chance that that
+ * multiple threads could get in here, and one thread could
+ * be scanning through the list of bits looking for a free
+ * one, but the free ones are always behind him, and other
+ * threads sneak in behind him and eat them before he can
+ * get to them, so that while there is always a free one, a
+ * very unlucky thread might be starved anyway, never able to
+ * beat the other threads. In reality, this happens so
+ * infrequently as to be indistinguishable from never.
+ */
- spin_lock_irqsave(&h->lock, flags);
+ loopcount = 0;
do {
i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
- if (i == h->nr_cmds) {
- spin_unlock_irqrestore(&h->lock, flags);
- return NULL;
- }
- } while (test_and_set_bit
- (i & (BITS_PER_LONG - 1),
- h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
- spin_unlock_irqrestore(&h->lock, flags);
+ if (i == h->nr_cmds)
+ i = 0;
+ loopcount++;
+ } while (test_and_set_bit(i & (BITS_PER_LONG - 1),
+ h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0 &&
+ loopcount < 10);
+
+ /* Thread got starved? We do not expect this to ever happen. */
+ if (loopcount >= 10)
+ return NULL;
c = h->cmd_pool + i;
memset(c, 0, sizeof(*c));
@@ -4679,13 +4692,10 @@ static struct CommandList *cmd_special_alloc(struct ctlr_info *h)
static void cmd_free(struct ctlr_info *h, struct CommandList *c)
{
int i;
- unsigned long flags;
i = c - h->cmd_pool;
- spin_lock_irqsave(&h->lock, flags);
clear_bit(i & (BITS_PER_LONG - 1),
h->cmd_pool_bits + (i / BITS_PER_LONG));
- spin_unlock_irqrestore(&h->lock, flags);
}
static void cmd_special_free(struct ctlr_info *h, struct CommandList *c)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 00/13] hpsa update
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
` (12 preceding siblings ...)
2014-11-14 23:27 ` [PATCH 13/13] hpsa: remove spin lock around command allocation Don Brace
@ 2014-11-20 15:55 ` Christoph Hellwig
2014-11-24 19:22 ` brace
13 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2014-11-20 15:55 UTC (permalink / raw)
To: Don Brace; +Cc: hch, webb.scales, james.bottomley, brace, linux-scsi
I've applied all patches to drivers-for-3.19.
If you could do me two favours:
- please look into sparse endianess warnings. This series introduces
a few new ones, but given that the area around it was full of it
I didn't pull it for that for now.
- please review the various hpsa fixes Tomas Henzl sent over the last
few weeks.
Thanks,
Christoph
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 00/13] hpsa update
2014-11-20 15:55 ` [PATCH 00/13] hpsa update Christoph Hellwig
@ 2014-11-24 19:22 ` brace
0 siblings, 0 replies; 16+ messages in thread
From: brace @ 2014-11-24 19:22 UTC (permalink / raw)
To: Christoph Hellwig, Don Brace
Cc: webb.scales@hp.com, james.bottomley@parallels.com,
linux-scsi@vger.kernel.org
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, November 20, 2014 9:55 AM
> To: Don Brace
> Cc: hch@infradead.org; webb.scales@hp.com;
> james.bottomley@parallels.com; brace; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 00/13] hpsa update
>
> I've applied all patches to drivers-for-3.19.
>
> If you could do me two favours:
>
> - please look into sparse endianess warnings. This series introduces
> a few new ones, but given that the area around it was full of it
> I didn't pull it for that for now.
> - please review the various hpsa fixes Tomas Henzl sent over the last
> few weeks.
>
> Thanks,
> Christoph
I created a patch for the extra sparse warnings and I have it in for a local review.
I will get it up soon. Most of the people in my area are off this week.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-11-24 19:43 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-14 23:26 [PATCH 00/13] hpsa update Don Brace
2014-11-14 23:26 ` [PATCH 01/13] hpsa: Clean up warnings from sparse Don Brace
2014-11-14 23:26 ` [PATCH 02/13] hpsa: remove dev_warn prints from RAID-1ADM Don Brace
2014-11-14 23:26 ` [PATCH 03/13] hpsa: fix a couple pci id table mistakes Don Brace
2014-11-14 23:26 ` [PATCH 04/13] hpsa: correct off-by-one sizing of chained SG block Don Brace
2014-11-14 23:26 ` [PATCH 05/13] hpsa: remove 'action required' phrasing Don Brace
2014-11-14 23:26 ` [PATCH 06/13] hpsa: fix allocation sizes for CISS_REPORT_LUNs commands Don Brace
2014-11-14 23:26 ` [PATCH 07/13] hpsa: fix endianness issue with scatter gather elements Don Brace
2014-11-14 23:27 ` [PATCH 08/13] hpsa: get rid of type/attribute/direction bit field where possible Don Brace
2014-11-14 23:27 ` [PATCH 09/13] hpsa: use atomics for commands_outstanding Don Brace
2014-11-14 23:27 ` [PATCH 10/13] hpsa: do not be so noisy about check conditions Don Brace
2014-11-14 23:27 ` [PATCH 11/13] hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation Don Brace
2014-11-14 23:27 ` [PATCH 12/13] hpsa: always call pci_set_master after pci_enable_device Don Brace
2014-11-14 23:27 ` [PATCH 13/13] hpsa: remove spin lock around command allocation Don Brace
2014-11-20 15:55 ` [PATCH 00/13] hpsa update Christoph Hellwig
2014-11-24 19:22 ` brace
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox