Re: [PATCH v3 34/57] scsi: libfc: Enable lock context analysis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incomplete lock context annotations for intermediate and explicitly locked functions, triggering static analysis warnings.
--

--- Patch [34]: [PATCH v3 34/57] scsi: libfc: Enable lock context analysis ---
commit 713891d004459388a5a60d9452963a9f124605ff
Author: Bart Van Assche <bvanassche@acm.org>

scsi: libfc: Enable lock context analysis

Document locking requirements with __must_hold().

> diff --git a/drivers/scsi/libfc/Makefile b/drivers/scsi/libfc/Makefile
> --- a/drivers/scsi/libfc/Makefile
> +++ b/drivers/scsi/libfc/Makefile
> @@ -1,6 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # $Id: Makefile
>  
> +CONTEXT_ANALYSIS := y
> +

[Severity: Low]
Does enabling context analysis here trigger compiler warnings for other
functions that are missing annotations?

For example, shouldn't fc_disc_stop_rports() and fc_exch_done_locked() be
annotated? They both explicitly require locks but appear to be missing the
__must_hold() annotations:

drivers/scsi/libfc/fc_disc.c:fc_disc_stop_rports() {
    ...
    lockdep_assert_held(&disc->disc_mutex);
    ...
}

[ ... ]

> diff --git a/drivers/scsi/libfc/fc_disc.c b/drivers/scsi/libfc/fc_disc.c
> --- a/drivers/scsi/libfc/fc_disc.c
> +++ b/drivers/scsi/libfc/fc_disc.c
> @@ -200,6 +200,7 @@ static void fc_disc_recv_req(struct fc_lport *lport, struct fc_frame *fp)
>   * @disc: The discovery object to be restarted
>   */
>  static void fc_disc_restart(struct fc_disc *disc)
> +	__must_hold(&disc->disc_mutex)
>  {

[Severity: Low]
Does fc_disc_recv_rscn_req() also need the __must_hold() annotation?

It calls this newly annotated fc_disc_restart() function, so without its own
annotation, could it break the analysis chain for its callers?

[ ... ]

> diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
> --- a/drivers/scsi/libfc/fc_fcp.c
> +++ b/drivers/scsi/libfc/fc_fcp.c
> @@ -1950,6 +1953,7 @@ EXPORT_SYMBOL(fc_queuecommand);
>   * The fcp packet lock must be held when calling.
>   */
>  static void fc_io_compl(struct fc_fcp_pkt *fsp)
> +	__must_hold(&fsp->scsi_pkt_lock)
>  {

[Severity: Low]
Does fc_fcp_complete_locked() also need to be annotated?

It calls fc_io_compl(), so lacking a __must_hold() annotation might cause a
context imbalance warning during analysis.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785451174.git.bvanassche@acm.org?part=34