All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Tyrel Datwyler" <tyreld@linux.ibm.com>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH v11 10/11] scsi: ibmvfc: register and use asynchronous sub CRQ for events
Date: Fri, 25 Sep 2026 05:09:03 +0000	[thread overview]
Message-ID: <20260925050903.EEEC21F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260925044900.414340-11-tyreld@linux.ibm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: double registration of scsi_scrqs and missing async_scrqs registration
--

commit d44dbe8300fc3639dbfcef8f1950443462606f5b
Author: Dave Marquardt <davemarq@linux.ibm.com>

scsi: ibmvfc: register and use asynchronous sub CRQ for events

This commit wires the async sub-CRQ into the sub-CRQ lifecycle so it is
allocated, registered, and freed alongside the SCSI channel queues.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -986,6 +987,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
>  	spin_unlock(vhost->crq.q_lock);
>  	spin_unlock_irqrestore(vhost->host->host_lock, flags);
>  
> +	ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
>  	ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
>  	ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
>  

[Severity: High]
Could this introduce a regression where scsi_scrqs is accidentally registered
twice instead of registering async_scrqs?

Because the first call was added as ibmvfc_reg_sub_crqs(vhost,
&vhost->scsi_scrqs), it appears to skip the async sub-CRQ registration
entirely. This leaves the async sub-CRQ permanently disabled after a CRQ
re-enablement, which could prevent asynchronous event handling (like FPINs)
after recovery.

Also, wouldn't the double registration of scsi_scrqs cause
ibmvfc_register_channel() to fail? If H_REG_SUB_CRQ returns an error for an
already registered channel, this will abort the registration and clear
vhost->do_enquiry = 0, functionally breaking multi-queue capabilities during
standard operational recovery paths.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925044900.414340-1-tyreld@linux.ibm.com?part=10

  reply	other threads:[~2026-09-25  5:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25  4:48 [PATCH v11 00/11] scsi: ibmvfc: make ibmvfc support FPIN messages Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 01/11] scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 02/11] scsi: ibmvfc: fix potential clobbering of rc after failed irq setup Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 03/11] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 04/11] scsi: ibmvfc: add basic FPIN support Tyrel Datwyler
2026-09-25  5:03   ` sashiko-bot
2026-09-25 16:16     ` Dave Marquardt
2026-09-25 16:36       ` Dave Marquardt
2026-09-28 14:11         ` Dave Marquardt
2026-09-25  4:48 ` [PATCH v11 05/11] scsi: ibmvfc: add NOOP command support Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 06/11] scsi: ibmvfc: add FPIN extended flag and async sub-CRQ queue handle Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 07/11] scsi: ibmvfc: extend async event handlers for async sub-CRQ events Tyrel Datwyler
2026-09-25  5:00   ` sashiko-bot
2026-09-28 15:45     ` Dave Marquardt
2026-09-25  4:48 ` [PATCH v11 08/11] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ Tyrel Datwyler
2026-09-25  4:48 ` [PATCH v11 09/11] scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ Tyrel Datwyler
2026-09-25  5:05   ` sashiko-bot
2026-09-28 15:55     ` Dave Marquardt
2026-09-25  4:48 ` [PATCH v11 10/11] scsi: ibmvfc: register and use asynchronous sub CRQ for events Tyrel Datwyler
2026-09-25  5:09   ` sashiko-bot [this message]
2026-09-28 22:30     ` Dave Marquardt
2026-09-29 15:33       ` Tyrel Datwyler
2026-09-25  4:49 ` [PATCH v11 11/11] scsi: ibmvfc: handle extended FPIN events Tyrel Datwyler

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=20260925050903.EEEC21F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tyreld@linux.ibm.com \
    /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.