Message ID | 20210830231305.6334-1-jsmart2021@gmail.com |
---|---|
State | New |
Headers | show |
Series | lpfc: Fix compilation errors on kernels with no CONFIG_DEBUG_FS | expand |
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@broadcom.com> > Signed-off-by: Justin Tee <justin.tee@broadcom.com> > Signed-off-by: James Smart <jsmart2021@gmail.com> 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@kernel.org> 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
On 9/7/2021 4:06 PM, Nathan Chancellor wrote: > 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@broadcom.com> >> Signed-off-by: Justin Tee <justin.tee@broadcom.com> >> Signed-off-by: James Smart <jsmart2021@gmail.com> > > 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@kernel.org> > > 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? Yep. It is simpler. > ... > > 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. NP. I'll take care of it. -- james
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: - 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(); rdata = lpfc_rport_data_from_scsi_device(cmnd->device);