[v2] eal: don't call RTE_LOG before init

Message ID 20190919133107.14147-1-stephen@networkplumber.org (mailing list archive)
State Rejected, archived
Delegated to: David Marchand
Headers
Series [v2] eal: don't call RTE_LOG before init |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-dpdk_compile_ovs success Compile Testing PASS
ci/iol-dpdk_compile_spdk success Compile Testing PASS
ci/iol-dpdk_compile success Compile Testing PASS
ci/intel-Performance success Performance Testing PASS
ci/mellanox-Performance success Performance Testing PASS

Commit Message

Stephen Hemminger Sept. 19, 2019, 1:31 p.m. UTC
  rte_init_alert is called before rte_log is initialized.
Therefore RTE_LOG() should not be used (only stderr).

For VFIO, it is initialized after rte_log_init therefore,
use RTE_LOG.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
  

Comments

David Marchand Oct. 16, 2019, 7:42 a.m. UTC | #1
On Thu, Sep 19, 2019 at 3:31 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> rte_init_alert is called before rte_log is initialized.

rte_eal_init_alert*

> Therefore RTE_LOG() should not be used (only stderr).

There should be nothing disastrous when calling RTE_LOG() before
rte_eal_log_init has been called.
RTE_LOG -> rte_vlog with rte_logs.file == default_log_stream == NULL
=> it should end up writing to stderr in such a case.

And I understand this is the issue you wanted to fix because we write
the same messages twice on stderr.

>
> For VFIO, it is initialized after rte_log_init therefore,
> use RTE_LOG.

What about all the other calls after the vfio setup step?
Now with this change, they would only be passed to stderr even if the
log subsystem is initialised.

Can we have a "log initialised" boolean here and use it in rte_eal_init_alert?

>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/linux/eal/eal.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 946222ccdb7a..6f54a8b2133f 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -950,7 +950,6 @@ static int rte_eal_vfio_setup(void)
>  static void rte_eal_init_alert(const char *msg)
>  {
>         fprintf(stderr, "EAL: FATAL: %s\n", msg);
> -       RTE_LOG(ERR, EAL, "%s\n", msg);
>  }
>
>  /*
> @@ -1175,7 +1174,7 @@ rte_eal_init(int argc, char **argv)
>
>  #ifdef VFIO_PRESENT
>         if (rte_eal_vfio_setup() < 0) {
> -               rte_eal_init_alert("Cannot init VFIO");
> +               RTE_LOG(ERR, EAL, "Cannot init VFIO\n");
>                 rte_errno = EAGAIN;
>                 rte_atomic32_clear(&run_once);
>                 return -1;
> --
> 2.17.1
>
  

Patch

diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 946222ccdb7a..6f54a8b2133f 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -950,7 +950,6 @@  static int rte_eal_vfio_setup(void)
 static void rte_eal_init_alert(const char *msg)
 {
 	fprintf(stderr, "EAL: FATAL: %s\n", msg);
-	RTE_LOG(ERR, EAL, "%s\n", msg);
 }
 
 /*
@@ -1175,7 +1174,7 @@  rte_eal_init(int argc, char **argv)
 
 #ifdef VFIO_PRESENT
 	if (rte_eal_vfio_setup() < 0) {
-		rte_eal_init_alert("Cannot init VFIO");
+		RTE_LOG(ERR, EAL, "Cannot init VFIO\n");
 		rte_errno = EAGAIN;
 		rte_atomic32_clear(&run_once);
 		return -1;