Re: [PATCH v3 2/2] scsi: libsas: terminate deferred commands on time out

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing function declaration for `ata_eh_schedule_deferred_qc_retry()` when `CONFIG_SATA_HOST` is disabled, leading to a build failure.
--

--- Patch [2]: [PATCH v3 2/2] scsi: libsas: terminate deferred commands on time out ---
commit 3af8343f458da773cfa43a85c2e86949acf28cc1
Author: Damien Le Moal <[email protected]>

scsi: libsas: terminate deferred commands on time out

If a command timeout occurs while we have a deferred non-NCQ command waiting
to be issued, the SCSI EH task is not immediately woken up. This patch avoids
this unnecessary wait by using the eh_timed_out SCSI host template operation.

> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index c83282733ec4c..6b62522bd0b2e 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
[ ... ]
> @@ -502,6 +502,23 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd)
>  }
>  EXPORT_SYMBOL_GPL(sas_eh_target_reset_handler);
>  
> +/*
> + * Handle deferred QCs in case of a command timeout.
> + * See ata_scsi_eh_timed_out() for details.
> + */
> +enum scsi_timeout_action sas_eh_timed_out(struct scsi_cmnd *cmd)
> +{
> +	struct domain_device *dev = NULL;
> +
> +	if (cmd)
> +		dev = cmd_to_domain_dev(cmd);
> +	if (dev && dev_is_sata(dev))
> +		ata_eh_schedule_deferred_qc_retry(dev->sata_dev.ap, cmd);
> +
> +	return SCSI_EH_NOT_HANDLED;
> +}
> +EXPORT_SYMBOL_GPL(sas_eh_timed_out);
> +
[ ... ]
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 327da43d74967..2ea7bfbdd867b 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1261,6 +1261,8 @@ extern int sata_link_hardreset(struct ata_link *link,
>  extern int sata_link_resume(struct ata_link *link, const unsigned int *params,
>  			    unsigned long deadline);
>  extern void ata_eh_analyze_ncq_error(struct ata_link *link);
> +void ata_eh_schedule_deferred_qc_retry(struct ata_port *ap,
> +				       struct scsi_cmnd *scmd);
>  #else

[Severity: Low]
Will this result in a build failure when CONFIG_SATA_HOST is disabled?

The declaration for ata_eh_schedule_deferred_qc_retry() is placed inside
this conditional block. When CONFIG_SATA_HOST is disabled, this declaration
is omitted, but the function is still called unconditionally by
sas_eh_timed_out() in sas_scsi_host.c (as well as ata_scsi_eh_timed_out()
in libata-scsi.c). 

Since modern compilers treat implicit function declarations as errors, could
this break the build under these configurations?

>  static inline const unsigned int *
>  sata_ehc_deb_timing(struct ata_eh_context *ehc)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2