[dpdk-dev] [PATCH] eal: remove redundant error output

[dpdk-dev] [PATCH] eal: remove redundant error output

[Stephen Hemminger]

The function rte_eal_init_alert ends up printing the same message
twice. Once via RTE_LOG and once to stderr. Remove the fprintf
to stderr since it is redundant.

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

diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 946222ccdb7a..076fb3cbde5f 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -949,7 +949,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);
 }
 
-- 
2.20.1

Re: [dpdk-dev] [PATCH] eal: remove redundant error output

[Aaron Conole]

Stephen Hemminger <stephen@networkplumber.org> writes:

> The function rte_eal_init_alert ends up printing the same message
> twice. Once via RTE_LOG and once to stderr. Remove the fprintf
> to stderr since it is redundant.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

This was originally added at your suggestion:

http://mails.dpdk.org/archives/dev/2017-January/056431.html

Because sometimes we have these alerts before logging is up (so the
RTE_LOG(...) won't show up, I gather).

Is it possible to have an either/or?

>  lib/librte_eal/linux/eal/eal.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 946222ccdb7a..076fb3cbde5f 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -949,7 +949,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);
>  }

[dpdk-dev] [PATCH v2] eal: don't call RTE_LOG before init

[Stephen Hemminger]

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

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

Re: [dpdk-dev] [PATCH v2] eal: don't call RTE_LOG before init

[David Marchand]

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
>


-- 
David Marchand