diff mbox

[dpdk-dev,v5,3/5] bus: introduce new log type for bus drivers

Message ID 1507548444-33959-4-git-send-email-jianfeng.tan@intel.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

Jianfeng Tan Oct. 9, 2017, 11:27 a.m. UTC 
  Introduce a new log type, RTE_LOGTYPE_BUS, for bus drivers. And
change fslmc to use this type for logging.

Suggested-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/bus/fslmc/fslmc_bus.c           |  9 +++----
 drivers/bus/fslmc/fslmc_logs.h          | 42 ++++-----------------------------
 drivers/bus/fslmc/fslmc_vfio.c          |  4 +---
 lib/librte_eal/common/eal_common_log.c  |  1 +
 lib/librte_eal/common/include/rte_log.h |  1 +
 5 files changed, 11 insertions(+), 46 deletions(-)

Comments

Shreyansh Jain Oct. 11, 2017, 6:54 a.m. UTC | #1 
Hello Jianfeng,

On Monday 09 October 2017 04:57 PM, Jianfeng Tan wrote:
> Introduce a new log type, RTE_LOGTYPE_BUS, for bus drivers. And
> change fslmc to use this type for logging.
> 
> Suggested-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>   drivers/bus/fslmc/fslmc_bus.c           |  9 +++----
>   drivers/bus/fslmc/fslmc_logs.h          | 42 ++++-----------------------------
>   drivers/bus/fslmc/fslmc_vfio.c          |  4 +---
>   lib/librte_eal/common/eal_common_log.c  |  1 +
>   lib/librte_eal/common/include/rte_log.h |  1 +
>   5 files changed, 11 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
> index 0a8229f..236ec3a 100644
> --- a/drivers/bus/fslmc/fslmc_bus.c
> +++ b/drivers/bus/fslmc/fslmc_bus.c
> @@ -34,7 +34,6 @@
>   #include <dirent.h>
>   #include <stdbool.h>
>   
> -#include <rte_log.h>
>   #include <rte_bus.h>
>   #include <rte_eal_memconfig.h>
>   #include <rte_malloc.h>
> @@ -42,11 +41,9 @@
>   #include <rte_memcpy.h>
>   #include <rte_ethdev.h>
>   
> -#include <rte_fslmc.h>
> -#include <fslmc_vfio.h>
> -
> -#define FSLMC_BUS_LOG(level, fmt, args...) \
> -	RTE_LOG(level, EAL, fmt "\n", ##args)
> +#include "rte_fslmc.h"
> +#include "fslmc_vfio.h"
> +#include "fslmc_logs.h"
>   
>   #define VFIO_IOMMU_GROUP_PATH "/sys/kernel/iommu_groups"
>   
> diff --git a/drivers/bus/fslmc/fslmc_logs.h b/drivers/bus/fslmc/fslmc_logs.h
> index 1f7c24b..dbf2281 100644
> --- a/drivers/bus/fslmc/fslmc_logs.h
> +++ b/drivers/bus/fslmc/fslmc_logs.h
> @@ -33,44 +33,12 @@
>   #ifndef _FSLMC_LOGS_H_
>   #define _FSLMC_LOGS_H_
>   
> -#define PMD_INIT_LOG(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
> +#include <rte_log.h>
>   
> -#ifdef RTE_LIBRTE_DPAA2_DEBUG_INIT
> -#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
> -#else
> -#define PMD_INIT_FUNC_TRACE() do { } while (0)
> -#endif
> +#define FSLMC_BUS_LOG(level, fmt, args...) \
> +	RTE_LOG(level, BUS, "%s(): " fmt "\n", __func__, ##args)
>   
> -#ifdef RTE_LIBRTE_DPAA2_DEBUG_RX
> -#define PMD_RX_LOG(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> -#else
> -#define PMD_RX_LOG(level, fmt, args...) do { } while (0)
> -#endif
> -
> -#ifdef RTE_LIBRTE_DPAA2_DEBUG_TX
> -#define PMD_TX_LOG(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> -#else
> -#define PMD_TX_LOG(level, fmt, args...) do { } while (0)
> -#endif
> -
> -#ifdef RTE_LIBRTE_DPAA2_DEBUG_TX_FREE
> -#define PMD_TX_FREE_LOG(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> -#else
> -#define PMD_TX_FREE_LOG(level, fmt, args...) do { } while (0)
> -#endif
> -
> -#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
> -#define PMD_DRV_LOG_RAW(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
> -#else
> -#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
> -#endif
> -
> -#define PMD_DRV_LOG(level, fmt, args...) \
> -	PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
> +#define FSLMC_VFIO_LOG(level, fmt, args...) \
> +	RTE_LOG(level, EAL, "%s(): " fmt "\n", __func__, ##args)

This change breaks the FSLMC bus driver. There are macros like 
PMD_DRV_LOG which are still in use in the code.
Before removing the above, those would have to be restructured.

I am already working on converting this logging into dynamic logging.
Can you skip this work until then? Does it block your work?

>   
>   #endif /* _FSLMC_LOGS_H_ */

<snip>
Jianfeng Tan Oct. 11, 2017, 10:42 a.m. UTC | #2 
On 10/11/2017 2:54 PM, Shreyansh Jain wrote:
> Hello Jianfeng,
>
> On Monday 09 October 2017 04:57 PM, Jianfeng Tan wrote:
>>
[...]
>> -#define PMD_DRV_LOG(level, fmt, args...) \
>> -    PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
>> +#define FSLMC_VFIO_LOG(level, fmt, args...) \
>> +    RTE_LOG(level, EAL, "%s(): " fmt "\n", __func__, ##args)
>
> This change breaks the FSLMC bus driver. There are macros like 
> PMD_DRV_LOG which are still in use in the code.
> Before removing the above, those would have to be restructured.

Just try to change all PMD_DRV_LOG in fslmc to FSLMC_VFIO_LOG. As you 
are working on that, I will drop it.

>
> I am already working on converting this logging into dynamic logging.
> Can you skip this work until then? Does it block your work?

Do you mean you are working on introducing a new log type for bus drivers?

Thanks,
Jianfeng
Shreyansh Jain Oct. 11, 2017, 11:20 a.m. UTC | #3 
On Wednesday 11 October 2017 04:12 PM, Tan, Jianfeng wrote:
> 
> 
> On 10/11/2017 2:54 PM, Shreyansh Jain wrote:
>> Hello Jianfeng,
>>
>> On Monday 09 October 2017 04:57 PM, Jianfeng Tan wrote:
>>>
> [...]
>>> -#define PMD_DRV_LOG(level, fmt, args...) \
>>> -    PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
>>> +#define FSLMC_VFIO_LOG(level, fmt, args...) \
>>> +    RTE_LOG(level, EAL, "%s(): " fmt "\n", __func__, ##args)
>>
>> This change breaks the FSLMC bus driver. There are macros like 
>> PMD_DRV_LOG which are still in use in the code.
>> Before removing the above, those would have to be restructured.
> 
> Just try to change all PMD_DRV_LOG in fslmc to FSLMC_VFIO_LOG. As you 
> are working on that, I will drop it. >
>>
>> I am already working on converting this logging into dynamic logging.
>> Can you skip this work until then? Does it block your work?
> 
> Do you mean you are working on introducing a new log type for bus drivers?

A dynamic log type using rte_log_register. So, "bus.fslmc", "net.dpaa2" 
and so on. I am not introducing LOGTYPE_BUS.

In past [1], I sent a patch for LOGTYPE_BUS but at that time I 
understood that dynamic logging is the preferred way.

[1] http://dpdk.org/dev/patchwork/patch/24478/


> 
> Thanks,
> Jianfeng
>
Jianfeng Tan Oct. 12, 2017, 2:14 a.m. UTC | #4 
On 10/11/2017 7:20 PM, Shreyansh Jain wrote:
> On Wednesday 11 October 2017 04:12 PM, Tan, Jianfeng wrote:
>>
>>
>> On 10/11/2017 2:54 PM, Shreyansh Jain wrote:
>>> Hello Jianfeng,
>>>
>>> On Monday 09 October 2017 04:57 PM, Jianfeng Tan wrote:
>>>>
>> [...]
>>>> -#define PMD_DRV_LOG(level, fmt, args...) \
>>>> -    PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
>>>> +#define FSLMC_VFIO_LOG(level, fmt, args...) \
>>>> +    RTE_LOG(level, EAL, "%s(): " fmt "\n", __func__, ##args)
>>>
>>> This change breaks the FSLMC bus driver. There are macros like 
>>> PMD_DRV_LOG which are still in use in the code.
>>> Before removing the above, those would have to be restructured.
>>
>> Just try to change all PMD_DRV_LOG in fslmc to FSLMC_VFIO_LOG. As you 
>> are working on that, I will drop it. >
>>>
>>> I am already working on converting this logging into dynamic logging.
>>> Can you skip this work until then? Does it block your work?
>>
>> Do you mean you are working on introducing a new log type for bus 
>> drivers?
>
> A dynamic log type using rte_log_register. So, "bus.fslmc", 
> "net.dpaa2" and so on. I am not introducing LOGTYPE_BUS.
>
> In past [1], I sent a patch for LOGTYPE_BUS but at that time I 
> understood that dynamic logging is the preferred way.
>
> [1] http://dpdk.org/dev/patchwork/patch/24478/

OK, that makes sense. I'll hold this change then. Thanks for your 
insight on this.

Thanks,
Jianfeng
diff mbox

Patch

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 0a8229f..236ec3a 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -34,7 +34,6 @@ 
 #include <dirent.h>
 #include <stdbool.h>
 
-#include <rte_log.h>
 #include <rte_bus.h>
 #include <rte_eal_memconfig.h>
 #include <rte_malloc.h>
@@ -42,11 +41,9 @@ 
 #include <rte_memcpy.h>
 #include <rte_ethdev.h>
 
-#include <rte_fslmc.h>
-#include <fslmc_vfio.h>
-
-#define FSLMC_BUS_LOG(level, fmt, args...) \
-	RTE_LOG(level, EAL, fmt "\n", ##args)
+#include "rte_fslmc.h"
+#include "fslmc_vfio.h"
+#include "fslmc_logs.h"
 
 #define VFIO_IOMMU_GROUP_PATH "/sys/kernel/iommu_groups"
 
diff --git a/drivers/bus/fslmc/fslmc_logs.h b/drivers/bus/fslmc/fslmc_logs.h
index 1f7c24b..dbf2281 100644
--- a/drivers/bus/fslmc/fslmc_logs.h
+++ b/drivers/bus/fslmc/fslmc_logs.h
@@ -33,44 +33,12 @@ 
 #ifndef _FSLMC_LOGS_H_
 #define _FSLMC_LOGS_H_
 
-#define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
+#include <rte_log.h>
 
-#ifdef RTE_LIBRTE_DPAA2_DEBUG_INIT
-#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-#else
-#define PMD_INIT_FUNC_TRACE() do { } while (0)
-#endif
+#define FSLMC_BUS_LOG(level, fmt, args...) \
+	RTE_LOG(level, BUS, "%s(): " fmt "\n", __func__, ##args)
 
-#ifdef RTE_LIBRTE_DPAA2_DEBUG_RX
-#define PMD_RX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
-#else
-#define PMD_RX_LOG(level, fmt, args...) do { } while (0)
-#endif
-
-#ifdef RTE_LIBRTE_DPAA2_DEBUG_TX
-#define PMD_TX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
-#else
-#define PMD_TX_LOG(level, fmt, args...) do { } while (0)
-#endif
-
-#ifdef RTE_LIBRTE_DPAA2_DEBUG_TX_FREE
-#define PMD_TX_FREE_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
-#else
-#define PMD_TX_FREE_LOG(level, fmt, args...) do { } while (0)
-#endif
-
-#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
-#define PMD_DRV_LOG_RAW(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
-#else
-#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
-#endif
-
-#define PMD_DRV_LOG(level, fmt, args...) \
-	PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
+#define FSLMC_VFIO_LOG(level, fmt, args...) \
+	RTE_LOG(level, EAL, "%s(): " fmt "\n", __func__, ##args)
 
 #endif /* _FSLMC_LOGS_H_ */
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 7831201..984ff84 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -63,9 +63,7 @@ 
 
 #include "portal/dpaa2_hw_pvt.h"
 #include "portal/dpaa2_hw_dpio.h"
-
-#define FSLMC_VFIO_LOG(level, fmt, args...) \
-	RTE_LOG(level, EAL, fmt "\n", ##args)
+#include "fslmc_logs.h"
 
 /** Pathname of FSL-MC devices directory. */
 #define SYSFS_FSL_MC_DEVICES "/sys/bus/fsl-mc/devices"
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index b62b0a6..93cb615 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -279,6 +279,7 @@  static const struct logtype logtype_strings[] = {
 	{RTE_LOGTYPE_CRYPTODEV,  "cryptodev"},
 	{RTE_LOGTYPE_EFD,        "efd"},
 	{RTE_LOGTYPE_EVENTDEV,   "eventdev"},
+	{RTE_LOGTYPE_BUS,        "bus"},
 	{RTE_LOGTYPE_USER1,      "user1"},
 	{RTE_LOGTYPE_USER2,      "user2"},
 	{RTE_LOGTYPE_USER3,      "user3"},
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index ec8dba7..c1bb4c7 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -87,6 +87,7 @@  extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_CRYPTODEV 17 /**< Log related to cryptodev. */
 #define RTE_LOGTYPE_EFD       18 /**< Log related to EFD. */
 #define RTE_LOGTYPE_EVENTDEV  19 /**< Log related to eventdev. */
+#define RTE_LOGTYPE_BUS       20 /**< Log related to bus. */
 
 /* these log types can be used in an application */
 #define RTE_LOGTYPE_USER1     24 /**< User-defined log type 1. */