diff mbox

[dpdk-dev,2/2] mempool/dpaa2: improving the alloc/free logging

Message ID 1495532028-9700-2-git-send-email-hemant.agrawal@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Hemant Agrawal May 23, 2017, 9:33 a.m. UTC 
  Debug logs are helpful for better debugging. Alloc
was having the logs, but logs were not present in free routines.

This patch add support for debug mode logs in free routine.
Also, changing the log category to DRV instead of TX.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Olivier Matz June 8, 2017, 10:08 a.m. UTC | #1 
Hi Hemant,

On Tue, 23 May 2017 15:03:48 +0530, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> Debug logs are helpful for better debugging. Alloc
> was having the logs, but logs were not present in free routines.
> 
> This patch add support for debug mode logs in free routine.
> Also, changing the log category to DRV instead of TX.
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
> index 60dd1c0..e00ed5d 100644
> --- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
> +++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
> @@ -309,8 +309,8 @@
>  
>  #ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
>  	alloc += n;
> -	PMD_TX_LOG(DEBUG, "Total = %d , req = %d done = %d",
> -		   alloc, count, n);
> +	PMD_DRV_LOG(DEBUG, "Total = %d , req = %d done = %d",
> +		    alloc, count, n);
>  #endif
>  	return 0;
>  }

Since we are in a mempool driver, we may not want to use PMD_*_LOG()
functions.

Also, I don't see where these macros are defined for this file, but maybe
I miss something that is right under my nose ;)

Anyway, I suggest to use your own macros for dpaa mempool instead,
and if possible:
  - avoid compilation options as much as possible
  - RTE_LOG() for logs that cannot occur in dataplane (they can be enabled
    dynamically)
  - RTE_LOG_DP() for dataplane logs: these are stripped at compilation
    time, unless your log level <= RTE_LOG_DP_LEVEL


> @@ -320,6 +320,9 @@
>  		  void * const *obj_table, unsigned int n)
>  {
>  	struct dpaa2_bp_info *bp_info;
> +#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
> +	static int freed;
> +#endif
>  
>  	bp_info = mempool_to_bpinfo(pool);
>  	if (!(bp_info->bp_list)) {
> @@ -329,6 +332,11 @@
>  	rte_dpaa2_mbuf_release(pool, obj_table, bp_info->bpid,
>  			   bp_info->meta_data_size, n);
>  
> +#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
> +	freed += n;
> +	PMD_DRV_LOG(DEBUG, "Total = %d , done = %d",
> +		    freed, n);
> +#endif
>  	return 0;
>  }
>
Hemant Agrawal June 22, 2017, 12:46 p.m. UTC | #2 
On 6/8/2017 3:38 PM, Olivier Matz wrote:
> Hi Hemant,
>
> On Tue, 23 May 2017 15:03:48 +0530, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>> Debug logs are helpful for better debugging. Alloc
>> was having the logs, but logs were not present in free routines.
>>
>> This patch add support for debug mode logs in free routine.
>> Also, changing the log category to DRV instead of TX.
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>  drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
>> index 60dd1c0..e00ed5d 100644
>> --- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
>> +++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
>> @@ -309,8 +309,8 @@
>>
>>  #ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
>>  	alloc += n;
>> -	PMD_TX_LOG(DEBUG, "Total = %d , req = %d done = %d",
>> -		   alloc, count, n);
>> +	PMD_DRV_LOG(DEBUG, "Total = %d , req = %d done = %d",
>> +		    alloc, count, n);
>>  #endif
>>  	return 0;
>>  }
>
> Since we are in a mempool driver, we may not want to use PMD_*_LOG()
> functions.
>
> Also, I don't see where these macros are defined for this file, but maybe
> I miss something that is right under my nose ;)
>
These are being currently being driven from the bus logs.

> Anyway, I suggest to use your own macros for dpaa mempool instead,
> and if possible:
>   - avoid compilation options as much as possible
>   - RTE_LOG() for logs that cannot occur in dataplane (they can be enabled
>     dynamically)
>   - RTE_LOG_DP() for dataplane logs: these are stripped at compilation
>     time, unless your log level <= RTE_LOG_DP_LEVEL
>
your suggestion is good. I will rework the whole file and submit it later.

>
>> @@ -320,6 +320,9 @@
>>  		  void * const *obj_table, unsigned int n)
>>  {
>>  	struct dpaa2_bp_info *bp_info;
>> +#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
>> +	static int freed;
>> +#endif
>>
>>  	bp_info = mempool_to_bpinfo(pool);
>>  	if (!(bp_info->bp_list)) {
>> @@ -329,6 +332,11 @@
>>  	rte_dpaa2_mbuf_release(pool, obj_table, bp_info->bpid,
>>  			   bp_info->meta_data_size, n);
>>
>> +#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
>> +	freed += n;
>> +	PMD_DRV_LOG(DEBUG, "Total = %d , done = %d",
>> +		    freed, n);
>> +#endif
>>  	return 0;
>>  }
>>
>
>
diff mbox

Patch

diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
index 60dd1c0..e00ed5d 100644
--- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
+++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c
@@ -309,8 +309,8 @@ 
 
 #ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
 	alloc += n;
-	PMD_TX_LOG(DEBUG, "Total = %d , req = %d done = %d",
-		   alloc, count, n);
+	PMD_DRV_LOG(DEBUG, "Total = %d , req = %d done = %d",
+		    alloc, count, n);
 #endif
 	return 0;
 }
@@ -320,6 +320,9 @@ 
 		  void * const *obj_table, unsigned int n)
 {
 	struct dpaa2_bp_info *bp_info;
+#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
+	static int freed;
+#endif
 
 	bp_info = mempool_to_bpinfo(pool);
 	if (!(bp_info->bp_list)) {
@@ -329,6 +332,11 @@ 
 	rte_dpaa2_mbuf_release(pool, obj_table, bp_info->bpid,
 			   bp_info->meta_data_size, n);
 
+#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
+	freed += n;
+	PMD_DRV_LOG(DEBUG, "Total = %d , done = %d",
+		    freed, n);
+#endif
 	return 0;
 }