diff mbox series

eal: fix prompt info when remap_segment failed

Message ID 20230529112155.11247-1-changfengnan@bytedance.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal: fix prompt info when remap_segment failed |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Functional fail Functional issues
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Fengnan Chang May 29, 2023, 11:21 a.m. UTC 
  when there is enough space for memseg, we should pormpt which
config to modify, not just print numbers.

Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
---
 lib/eal/linux/eal_memory.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

David Marchand June 12, 2023, 1:09 p.m. UTC | #1 
On Mon, May 29, 2023 at 1:22 PM Fengnan Chang
<changfengnan@bytedance.com> wrote:
>
> when there is enough space for memseg, we should pormpt which
> config to modify, not just print numbers.

I think you want to explain the case when there is *not* enough
objects declared in the static configuration.
Could you rephrase and fix the typos?


>
> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> ---
>  lib/eal/linux/eal_memory.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
> index 0876974631..974db901b7 100644
> --- a/lib/eal/linux/eal_memory.c
> +++ b/lib/eal/linux/eal_memory.c
> @@ -716,9 +716,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
>                 break;
>         }
>         if (msl_idx == RTE_MAX_MEMSEG_LISTS) {
> -               RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n",
> -                               RTE_STR(RTE_MAX_MEMSEG_PER_TYPE),
> -                               RTE_STR(RTE_MAX_MEM_MB_PER_TYPE));
> +               RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST "
> +                               "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n");

This same log is in FreeBSD, so I guess we want to update it too.
Burakov, Anatoly June 13, 2023, 10:59 a.m. UTC | #2 
On 5/29/2023 12:21 PM, Fengnan Chang wrote:
> when there is enough space for memseg, we should pormpt which
> config to modify, not just print numbers.
> 
> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> ---
>   lib/eal/linux/eal_memory.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
> index 0876974631..974db901b7 100644
> --- a/lib/eal/linux/eal_memory.c
> +++ b/lib/eal/linux/eal_memory.c
> @@ -716,9 +716,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
>   		break;
>   	}
>   	if (msl_idx == RTE_MAX_MEMSEG_LISTS) {
> -		RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n",
> -				RTE_STR(RTE_MAX_MEMSEG_PER_TYPE),
> -				RTE_STR(RTE_MAX_MEM_MB_PER_TYPE));
> +		RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST "
> +				"RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n");
>   		return -1;

This is a problem with RTE_STR macro - replacing this with _RTE_STR will 
yield expected results (write out the RTE_MAX_MEMSEG_PER_TYPE as 
string). I'm not sure using _RTE_STR will be the correct solution 
though, because it's prefixed with an underscore (implying it should not 
be used directly), but I'm also not sure about writing out string 
literals like that explicitly.
Fengnan Chang June 15, 2023, 6:44 a.m. UTC | #3 
David Marchand <david.marchand@redhat.com> 于2023年6月12日周一 21:09写道：
>
> On Mon, May 29, 2023 at 1:22 PM Fengnan Chang
> <changfengnan@bytedance.com> wrote:
> >
> > when there is enough space for memseg, we should pormpt which
> > config to modify, not just print numbers.
>
> I think you want to explain the case when there is *not* enough
> objects declared in the static configuration.
> Could you rephrase and fix the typos?
>
Uhn, this is a careless mistake, I'll fix it. :)
>
> >
> > Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> > ---
> >  lib/eal/linux/eal_memory.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
> > index 0876974631..974db901b7 100644
> > --- a/lib/eal/linux/eal_memory.c
> > +++ b/lib/eal/linux/eal_memory.c
> > @@ -716,9 +716,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
> >                 break;
> >         }
> >         if (msl_idx == RTE_MAX_MEMSEG_LISTS) {
> > -               RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n",
> > -                               RTE_STR(RTE_MAX_MEMSEG_PER_TYPE),
> > -                               RTE_STR(RTE_MAX_MEM_MB_PER_TYPE));
> > +               RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST "
> > +                               "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n");
>
> This same log is in FreeBSD, so I guess we want to update it too.
>
>
> --
> David Marchand
>
Fengnan Chang June 15, 2023, 6:45 a.m. UTC | #4 
Burakov, Anatoly <anatoly.burakov@intel.com> 于2023年6月13日周二 19:00写道：
>
> On 5/29/2023 12:21 PM, Fengnan Chang wrote:
> > when there is enough space for memseg, we should pormpt which
> > config to modify, not just print numbers.
> >
> > Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> > ---
> >   lib/eal/linux/eal_memory.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
> > index 0876974631..974db901b7 100644
> > --- a/lib/eal/linux/eal_memory.c
> > +++ b/lib/eal/linux/eal_memory.c
> > @@ -716,9 +716,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
> >               break;
> >       }
> >       if (msl_idx == RTE_MAX_MEMSEG_LISTS) {
> > -             RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n",
> > -                             RTE_STR(RTE_MAX_MEMSEG_PER_TYPE),
> > -                             RTE_STR(RTE_MAX_MEM_MB_PER_TYPE));
> > +             RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST "
> > +                             "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n");
> >               return -1;
>
> This is a problem with RTE_STR macro - replacing this with _RTE_STR will
> yield expected results (write out the RTE_MAX_MEMSEG_PER_TYPE as
> string). I'm not sure using _RTE_STR will be the correct solution
> though, because it's prefixed with an underscore (implying it should not
> be used directly), but I'm also not sure about writing out string
> literals like that explicitly.
IMO, printing strings explicitly is better.
>
> --
> Thanks,
> Anatoly
>
diff mbox series

Patch

diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
index 0876974631..974db901b7 100644
--- a/lib/eal/linux/eal_memory.c
+++ b/lib/eal/linux/eal_memory.c
@@ -716,9 +716,8 @@  remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
 		break;
 	}
 	if (msl_idx == RTE_MAX_MEMSEG_LISTS) {
-		RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n",
-				RTE_STR(RTE_MAX_MEMSEG_PER_TYPE),
-				RTE_STR(RTE_MAX_MEM_MB_PER_TYPE));
+		RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST "
+				"RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n");
 		return -1;
 	}