On Mon, Aug 30, 2021 at 04:13:05PM -0700, James Smart wrote:
> The Kernel test robot flagged the following warning:
> ".../lpfc_init.c:7788:35: error: 'struct lpfc_sli4_hba' has no member
> named 'c_stat'"
>
> Reviewing this issue highlighted that one of the recent patches caused
> the driver to no longer compile cleanly if CONFIG_DEBUG_FS is not set.
>
> Correct the different areas that are failing to compile.
>
> Fixes: 02243836ad6f ("scsi: lpfc: Add support for the CM framework")
> Co-developed-by: Justin Tee <justin.tee@xxxxxxxxxxxx>
> Signed-off-by: Justin Tee <justin.tee@xxxxxxxxxxxx>
> Signed-off-by: James Smart <jsmart2021@xxxxxxxxx>
I got bit by this in certain configurations, it would be helpful to get
this into mainline sooner rather than later.
Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Couple of comments below.
> ---
> drivers/scsi/lpfc/lpfc_init.c | 12 ++++++++----
> drivers/scsi/lpfc/lpfc_nvme.c | 2 --
> drivers/scsi/lpfc/lpfc_scsi.c | 4 ----
> 3 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index d3f1fa38269f..a6127a51b4fe 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -8254,7 +8254,11 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
> lpfc_printf_log(phba, KERN_ERR, LOG_TRACE_EVENT,
> "3331 Failed allocating per cpu cgn stats\n");
> rc = -ENOMEM;
> - goto out_free_hba_hdwq_info;
> +#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
> + goto out_free_hba_hdwq_stat;
> +#else
> + goto out_free_hba_idle_stat;
> +#endif
> }
>
> /*
> @@ -8276,12 +8280,12 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
>
> return 0;
>
> -out_free_hba_hdwq_info:
Wouldn't it be simpler to just move the ifdef up one line and the endif
down one line to avoid the ifdef in the first hunk?
> - free_percpu(phba->sli4_hba.c_stat);
> #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
> +out_free_hba_hdwq_stat:
> + free_percpu(phba->sli4_hba.c_stat);
> +#endif
> out_free_hba_idle_stat:
> kfree(phba->sli4_hba.idle_stat);
> -#endif
> out_free_hba_eq_info:
> free_percpu(phba->sli4_hba.eq_info);
> out_free_hba_cpu_map:
> diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
> index 73a3568ff17e..479b3eed6208 100644
> --- a/drivers/scsi/lpfc/lpfc_nvme.c
> +++ b/drivers/scsi/lpfc/lpfc_nvme.c
> @@ -1489,9 +1489,7 @@ lpfc_nvme_fcp_io_submit(struct nvme_fc_local_port *pnvme_lport,
> struct lpfc_nvme_qhandle *lpfc_queue_info;
> struct lpfc_nvme_fcpreq_priv *freqpriv;
> struct nvme_common_command *sqe;
> -#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
> uint64_t start = 0;
> -#endif
>
> /* Validate pointers. LLDD fault handling with transport does
> * have timing races.
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index 0fde1e874c7a..dae5cc03e8c2 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -5578,12 +5578,8 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
> struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
> int err, idx;
> u8 *uuid = NULL;
> -#ifdef CONFIG_SCSI_LPFC_DEBUG_FS
> uint64_t start = 0L;
>
> - if (phba->ktime_on)
> - start = ktime_get_ns();
> -#endif
> start = ktime_get_ns();
Someone is probably going to come along and complain that the 0L is a
dead store. I would remove the assignment at the least but it might be
worth combining the two lines.
> rdata = lpfc_rport_data_from_scsi_device(cmnd->device);
>
> --
> 2.26.2
