diff mbox series

lpfc: Fix compilation errors on kernels with no CONFIG_DEBUG_FS

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

Commit Message

James Smart Aug. 30, 2021, 11:13 p.m. UTC
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>
---
 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(-)

Comments

Nathan Chancellor Sept. 7, 2021, 11:06 p.m. UTC | #1
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
James Smart Sept. 8, 2021, 5:09 a.m. UTC | #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 mbox series

Patch

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);