Fix CommandList alignment issues for old and
unsupported controllers.
This patch fixes the alignment issue reported for patch
'commit f749d8b7a989
("scsi: hpsa: Correct dev cmds outstanding for retried cmds")'
reported in https://lkml.org/lkml/2021/3/3/243
Patch 'commit f749d8b7a989
("scsi: hpsa: Correct dev cmds outstanding for retried cmds")'
removed an 'int' member field "abort_pending" and replaced it with
a 'bool' field "retry_pending". This was tested and verified both
by me and several others outside of Microchip.
* It should be noted that the patch changed the alignment of an
atomic_t shown in my pahole analysis below.
However, when the cciss block driver was removed from the
kernel, some old legacy cciss controllers were added into the
hpsa driver in patch 'commit 135ae6edeb51
("scsi: hpsa: add support for legacy boards")'
Changing the field from 'int' to 'bool' caused a command alignment
issue on these legacy controllers (reported on IA64 architectures)
but went unnoticed because we no longer have any functional test
systems supporting these controllers and also these controller
are not included in our out-of-box driver.
This patch corrects the CommandList alignment issue on
the legacy controllers by changing the 'bool' back to
an 'int'. I ran pahole against the original driver, current driver,
and on the driver built using the updated patch to verify
the alignment.
I changed setting retry_pending in the driver accordingly.
Before patch 'f749d8b7a989
("scsi: hpsa: Correct dev cmds outstanding for retried cmds")
/* --- cacheline 10 boundary (640 bytes) was 4 bytes ago --- */
struct hpsa_scsi_dev_t * phys_disk; /* 644 8 */
int abort_pending; /* 652 4 */
struct hpsa_scsi_dev_t * device; /* 656 8 */
atomic_t refcount; /* 664 4 */
/* size: 768, cachelines: 12, members: 16 */
/* padding: 100 */
} __attribute__((__aligned__(128)));
Current patch 'f749d8b7a989
("scsi: hpsa: Correct dev cmds outstanding for retried cmds")
/* --- cacheline 10 boundary (640 bytes) was 4 bytes ago --- */
struct hpsa_scsi_dev_t * phys_disk; /* 644 8 */
bool retry_pending; /* 652 1 */
struct hpsa_scsi_dev_t * device; /* 653 8 */
atomic_t refcount; /* 661 4 */
/* size: 768, cachelines: 12, members: 16 */
/* padding: 103 */
} __attribute__((__aligned__(128)));
After proposed patch applied:
/* --- cacheline 10 boundary (640 bytes) was 4 bytes ago --- */
struct hpsa_scsi_dev_t * phys_disk; /* 644 8 */
int retry_pending; /* 652 4 */
struct hpsa_scsi_dev_t * device; /* 656 8 */
atomic_t refcount; /* 664 4 */
/* size: 768, cachelines: 12, members: 16 */
/* padding: 100 */
} __attribute__((__aligned__(128)));
Reported-by: Sergei Trofimovich <slyich@xxxxxxxxx>
Tested-by: Sergei Trofimovich <slyich@xxxxxxxxx>
Reviewed-by: Scott Benesh <scott.benesh@xxxxxxxxxxxxx>
Reviewed-by: Mike McGowen <mike.mcgowen@xxxxxxxxxxxxx>
Signed-off-by: Don Brace <don.brace@xxxxxxxxxxxxx>
---
drivers/scsi/hpsa.c | 10 +++++-----
drivers/scsi/hpsa_cmd.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 38369766511c..96f26e250dae 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -5593,7 +5593,7 @@ static int hpsa_ioaccel_submit(struct ctlr_info *h,
c->scsi_cmd = cmd;
c->device = dev;
if (retry) /* Resubmit but do not increment device->commands_outstanding. */
- c->retry_pending = true;
+ c->retry_pending = 1;
rc = hpsa_scsi_ioaccel_raid_map(h, c);
if (rc < 0) /* scsi_dma_map failed. */
rc = SCSI_MLQUEUE_HOST_BUSY;
@@ -5603,7 +5603,7 @@ static int hpsa_ioaccel_submit(struct ctlr_info *h,
c->scsi_cmd = cmd;
c->device = dev;
if (retry) /* Resubmit but do not increment device->commands_outstanding. */
- c->retry_pending = true;
+ c->retry_pending = 1;
rc = hpsa_scsi_ioaccel_direct_map(h, c);
if (rc < 0) /* scsi_dma_map failed. */
rc = SCSI_MLQUEUE_HOST_BUSY;
@@ -5661,7 +5661,7 @@ static void hpsa_command_resubmit_worker(struct work_struct *work)
* Note: hpsa_ciss_submit does not zero out the command fields like
* ioaccel submit does.
*/
- c->retry_pending = true;
+ c->retry_pending = 1;
if (hpsa_ciss_submit(c->h, c, cmd, dev)) {
/*
* If we get here, it means dma mapping failed. Try
@@ -6169,7 +6169,7 @@ static struct CommandList *cmd_tagged_alloc(struct ctlr_info *h,
* This is a new command obtained from queue_command so
* there have not been any driver initiated retry attempts.
*/
- c->retry_pending = false;
+ c->retry_pending = 0;
return c;
}
@@ -6243,7 +6243,7 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
* cmd_alloc is for "internal" commands and they are never
* retried.
*/
- c->retry_pending = false;
+ c->retry_pending = 0;
return c;
}
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index d126bb877250..e86af4e9eef0 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -448,7 +448,7 @@ struct CommandList {
*/
struct hpsa_scsi_dev_t *phys_disk;
- bool retry_pending;
+ int retry_pending;
struct hpsa_scsi_dev_t *device;
atomic_t refcount; /* Must be last to avoid memset in hpsa_cmd_init() */
} __aligned(COMMANDLIST_ALIGNMENT);
