All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "David Strahan" <david.strahan@microchip.com>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] scsi: smartpqi: Fix AIO retry marker cleared by SCSI core between dispatches
Date: Wed, 22 Jul 2026 20:39:37 +0000	[thread overview]
Message-ID: <20260722203937.787DA1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260722202327.5242-3-david.strahan@microchip.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver-private retry marker (`this_residual`) is leaked across block layer tag reuses if a command completing with `DID_IMM_RETRY` is terminally completed by the SCSI midlayer. This permanently poisons the tag, disabling AIO bypass for future requests and causing the driver to erroneously offline healthy physical drives on routine errors.
--

--- Patch [1]: [PATCH] scsi: smartpqi: Fix AIO retry marker cleared by SCSI core between dispatches ---
commit b11e42cf8728d0b2c04d732af5bc5bfb1648bc25
Author: David Strahan <David.Strahan@microchip.com>

scsi: smartpqi: Fix AIO retry marker cleared by SCSI core between dispatches

This patch registers a per-command initialization callback to prevent the
SCSI core from clearing driver private data between dispatches. It allows an
AIO retry marker (this_residual) to survive requeues, with the driver taking
over the responsibility of clearing the marker on terminal completions.

> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c

[ ... ]

> @@ -5958,6 +5964,17 @@ void pqi_prep_for_scsi_done(struct scsi_cmnd *scmd)
>  	struct pqi_scsi_dev *device;
>  	struct completion *wait;
>  
> +	/*
> +	 * Clear the AIO-retry marker on final completion so the tag
> +	 * starts clean on its next dispatch.  On DID_IMM_RETRY leave
> +	 * it intact: pqi_aio_io_complete() sets DID_IMM_RETRY and
> +	 * bumps the marker to steer the requeue onto the RAID path,
> +	 * and pqi_process_raid_io_error() consumes the non-zero
> +	 * marker to offline a misbehaving drive.
> +	 */
> +	if (host_byte(scmd->result) != DID_IMM_RETRY)
> +		pqi_cmd_priv(scmd)->this_residual = 0;
> +

[Severity: High]
Does this code risk leaking the this_residual marker across unrelated
requests that reuse the same blk-mq tag?

If a command completing with DID_IMM_RETRY is terminally completed by the
SCSI midlayer instead of being requeued (for instance, if
scsi_cmd_runtime_exceeced(cmd) overrides the disposition to SUCCESS in
scsi_lib.c:scsi_complete()), the driver-private command data will not be
cleared here.

Because this patch disables the SCSI core's per-dispatch zeroing via
pqi_init_cmd_priv(), the stale this_residual > 0 marker would remain on
the tag.

If that poisoned tag is reused for a new request targeting a physical drive,
and the new request encounters a routine SCSI error (like ILLEGAL_REQUEST),
would pqi_process_raid_io_error() misinterpret the stale marker as a failed
AIO retry and erroneously call pqi_take_device_offline() on a healthy
physical drive?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722202327.5242-3-david.strahan@microchip.com?part=1

  reply	other threads:[~2026-07-22 20:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 20:23 [PATCH] scsi: smartpqi: add new pci device-ids David Strahan
2026-07-22 20:23 ` [PATCH] scsi: smartpqi: add support for CCISS_BIG_PASSTHRU ioctl David Strahan
2026-07-22 20:39   ` sashiko-bot
2026-07-22 20:23 ` [PATCH] scsi: smartpqi: Fix AIO retry marker cleared by SCSI core between dispatches David Strahan
2026-07-22 20:39   ` sashiko-bot [this message]
2026-07-22 20:23 ` [PATCH] scsi: smartpqi: update version to 2.1.42-011 David Strahan
2026-07-22 22:48 ` [PATCH] scsi: smartpqi: add new pci device-ids (please disregard, duplicate) David Strahan

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=20260722203937.787DA1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=david.strahan@microchip.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.