All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: Guenter Roeck <linux@roeck-us.net>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ata: libata-eh: Fix detection of deferred qc timeouts
Date: Fri, 6 Mar 2026 09:36:57 +0100	[thread overview]
Message-ID: <aaqSKaxrA7QKmZi7@ryzen> (raw)
In-Reply-To: <58d258e3-c76e-41da-bb3b-7fa0cde9d8a8@kernel.org>

On Fri, Mar 06, 2026 at 12:33:21PM +0900, Damien Le Moal wrote:
> On 3/6/26 11:48, Guenter Roeck wrote:
> > If the `ata_qc_for_each_raw()` loop finishes without finding a matching
> > `scmd`, `qc` will hold a pointer to the last element examined, which is
> > accociated with `i == ATA_MAX_QUEUE - 1`. This element can match
> > `ap->deferred_qc`. If that happens, the condition `qc == ap->deferred_qc`
> > evaluates to true despite the loop not breaking on a scmd match.
> > In that case, the error handler mistakenly intercepts a command that
> > completed normally after an unrelated SCSI timeout, returning a timeout
> > error instead of success.
> > 
> > Fix the problem by checking for i < ATA_MAX_QUEUE in addition to
> > qc == ap->deferred_qc.
> > 
> > The problem was found by an experimental code review agent based on
> > gemini-3.1-pro while reviewing backports into v6.18.y.
> 
> Please remove all the quotes:
> 
> If the ata_qc_for_each_raw() loop finishes without finding a matching SCSI
> command for any QC, the variable qc will hold a pointer to the last element
> examined, which has the tag i == ATA_MAX_QUEUE - 1. This qc can match the port
> deferred QC (ap->deferred_qc).
> 
> If that happens, the condition qc == ap->deferred_qc evaluates to true despite
> the loop not breaking with a match on the SCSI command for this QC. In that
> case, the error handler mistakenly intercepts a command that not completed yet

Could even write:
"that has not been issued yet"

(while it is true that it has not been completed yet, it has not even been
issued to the drive yet.)


> and that has not timed out, and thus erroneously returning a timeout error.
> 
> Fix the problem by checking for i < ATA_MAX_QUEUE in addition to
> qc == ap->deferred_qc.
> 
> The problem was found by an experimental code review agent based on
> gemini-3.1-pro while reviewing backports into v6.18.y.
> 
> > Assisted-by: Gemini:gemini-3.1-pro
> > Cc: Damien Le Moal <dlemoal@kernel.org>
> > Cc: Niklas Cassel <cassel@kernel.org>
> > Fixes: eddb98ad9364 ("ata: libata-eh: correctly handle deferred qc timeouts")
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> With the above, feel free to add:
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> 
> Thanks !
> 
> -- 
> Damien Le Moal
> Western Digital Research

  reply	other threads:[~2026-03-06  8:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-06  2:48 [PATCH] ata: libata-eh: Fix detection of deferred qc timeouts Guenter Roeck
2026-03-06  3:33 ` Damien Le Moal
2026-03-06  8:36   ` Niklas Cassel [this message]
2026-03-06  9:05 ` Niklas Cassel
2026-03-06 15:34   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aaqSKaxrA7QKmZi7@ryzen \
    --to=cassel@kernel.org \
    --cc=dlemoal@kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.