Thread (5 messages) 5 messages, 4 authors, 2021-03-05

Re: [bisected] 5.12-rc1 hpsa regression: "scsi: hpsa: Correct dev cmds outstanding for retried cmds" breaks hpsa P600

From: Sergei Trofimovich <hidden>
Date: 2021-03-03 15:05:17
Also in: lkml

On Wed, 3 Mar 2021 00:22:36 +0000
Sergei Trofimovich [off-list ref] wrote:
On Tue, 2 Mar 2021 23:31:32 +0100
John Paul Adrian Glaubitz [off-list ref] wrote:
quoted
Hi Sergei!

On 3/2/21 11:26 PM, Sergei Trofimovich wrote:  
quoted
Gave v5.12-rc1 a try today and got a similar boot failure around
hpsa queue initialization, but my failure is later:
    https://dev.gentoo.org/~slyfox/configs/guppy-dmesg-5.12-rc1
Maybe I get different error because I flipped on most debugging
kernel options :)

Looks like 'ERROR: Invalid distance value range' while being
very scary are harmless. It's just a new spammy way for kernel
to report lack of NUMA config on the machine (no SRAT and SLIT
ACPI tables).

At least I get hpsa detected on PCI bus. But I guess it's discovered
configuration is very wrong as I get unaligned accesses:
    [   19.811570] kernel unaligned access to 0xe000000105dd8295, ip=0xa000000100b874d1

Bisecting now.    
Sounds good. I guess we should get Jens' fix for the signal regression
merged as well as your two fixes for strace.  
"bisected" (cheated halfway through) and verified that reverting
f749d8b7a9896bc6e5ffe104cc64345037e0b152 makes rx3600 boot again.

CCing authors who might be able to help us here.

commit f749d8b7a9896bc6e5ffe104cc64345037e0b152
Author: Don Brace [off-list ref]
Date:   Mon Feb 15 16:26:57 2021 -0600

    scsi: hpsa: Correct dev cmds outstanding for retried cmds

    Prevent incrementing device->commands_outstanding for ioaccel command
    retries that are driver initiated.  If the command goes through the retry
    path, the device->commands_outstanding counter has already accounted for
    the number of commands outstanding to the device.  Only commands going
    through function hpsa_cmd_resolve_events decrement this counter.

     - ioaccel commands go to either HBA disks or to logical volumes comprised
       of SSDs.

    The extra increment is causing device resets to hang.

     - Resets wait for all device outstanding commands to complete before
       returning.

    Replace unused field abort_pending with retry_pending. This is a
    maintenance driver so these changes have the least impact/risk.

    Link: https://lore.kernel.org/r/161342801747.29388.13045495968308188518.stgit@brunhilda (local)
    Tested-by: Joe Szczypek [off-list ref]
    Reviewed-by: Scott Benesh [off-list ref]
    Reviewed-by: Scott Teel [off-list ref]
    Reviewed-by: Tomas Henzl [off-list ref]
    Signed-off-by: Don Brace [off-list ref]
    Signed-off-by: Martin K. Petersen [off-list ref]

Don, do you happen to know why this patch caused some controller init failure
for device
    14:01.0 RAID bus controller: Hewlett-Packard Company Smart Array P600
?

Boot failure: https://dev.gentoo.org/~slyfox/configs/guppy-dmesg-5.12-rc1
Boot success: https://dev.gentoo.org/~slyfox/configs/guppy-dmesg-5.12-rc1-good

The difference between the two boots is 
f749d8b7a9896bc6e5ffe104cc64345037e0b152 reverted on top of 5.12-rc1
in -good case.

Looks like hpsa controller fails to initialize in bad case (could be a race?).
Also CCing hpsa maintainer mailing lists.

Looking more into the suspect commit
    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f749d8b7a9896bc6e5ffe104cc64345037e0b152
it roughly does the:
@@ -448,7 +448,7 @@ struct CommandList {
 	 */
 	struct hpsa_scsi_dev_t *phys_disk;
 
-	int abort_pending;
+	bool retry_pending;
 	struct hpsa_scsi_dev_t *device;
 	atomic_t refcount; /* Must be last to avoid memset in hpsa_cmd_init() */
 } __aligned(COMMANDLIST_ALIGNMENT);
...
@@ -1151,7 +1151,10 @@ static void __enqueue_cmd_and_start_io(struct ctlr_info *h,
 {
        dial_down_lockup_detection_during_fw_flash(h, c);
        atomic_inc(&h->commands_outstanding);
-       if (c->device)
+       /*
+        * Check to see if the command is being retried.
+        */
+       if (c->device && !c->retry_pending)
                atomic_inc(&c->device->commands_outstanding);
But I don't immediately see anything wrong with it.

-- 

  Sergei
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help