[LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Pengfei Xu]

On Intel sapphire rapids server, BIOS could allocate one memory block for CXL
node when the server boot up, and this node "MemUsed" is 0 when CXL is not
used like as follow:
"
cat /sys/devices/system/node/node2/meminfo
Node 2 MemTotal:        4194304 kB
Node 2 MemFree:         4194304 kB
Node 2 MemUsed:               0 kB
...
"

And it caused get_mempolicy01/02 and set_mempolicy01/02/03/04 cases to fail
like as follow sample:
"
tag=get_mempolicy01 stime=1683272855
cmdline="get_mempolicy01"
contacts=""
analysis=exit
<<<test_output>>>
incrementing stop
tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
tst_numa.c:200: TINFO: Found 3 NUMA memory nodes
tst_numa.c:165: TWARN: Failed to parse '/sys/devices/system/node/node2/meminfo'
get_mempolicy01.c:188: TINFO: test #1: policy: MPOL_DEFAULT, no target
get_mempolicy01.c:191: TPASS: policy: MPOL_DEFAULT, no target passed
get_mempolicy01.c:188: TINFO: test #2: policy: MPOL_BIND
get_mempolicy01.c:191: TPASS: policy: MPOL_BIND passed
get_mempolicy01.c:188: TINFO: test #3: policy: MPOL_INTERLEAVE
get_mempolicy01.c:191: TPASS: policy: MPOL_INTERLEAVE passed
get_mempolicy01.c:188: TINFO: test #4: policy: MPOL_PREFERRED, no target
get_mempolicy01.c:191: TPASS: policy: MPOL_PREFERRED, no target passed
get_mempolicy01.c:188: TINFO: test #5: policy: MPOL_PREFERRED
get_mempolicy01.c:191: TPASS: policy: MPOL_PREFERRED passed
get_mempolicy01.c:188: TINFO: test #6: policy: MPOL_DEFAULT, flags: MPOL_F_ADDR, no target
get_mempolicy01.c:191: TPASS: policy: MPOL_DEFAULT, flags: MPOL_F_ADDR, no target passed
get_mempolicy01.c:188: TINFO: test #7: policy: MPOL_BIND, flags: MPOL_F_ADDR
get_mempolicy01.c:191: TPASS: policy: MPOL_BIND, flags: MPOL_F_ADDR passed
get_mempolicy01.c:188: TINFO: test #8: policy: MPOL_INTERLEAVE, flags: MPOL_F_ADDR
get_mempolicy01.c:191: TPASS: policy: MPOL_INTERLEAVE, flags: MPOL_F_ADDR passed
get_mempolicy01.c:188: TINFO: test #9: policy: MPOL_PREFERRED, flags: MPOL_F_ADDR, no target
get_mempolicy01.c:191: TPASS: policy: MPOL_PREFERRED, flags: MPOL_F_ADDR, no target passed
get_mempolicy01.c:188: TINFO: test #10: policy: MPOL_PREFERRED, flags: MPOL_F_ADDR
get_mempolicy01.c:191: TPASS: policy: MPOL_PREFERRED, flags: MPOL_F_ADDR passed

Summary:
passed   10
failed   0
broken   0
skipped  0
warnings 1
...
--------                                           ------     ----------
get_mempolicy01                                    FAIL       4
"
So fixed the fake failure when CXL node memory is not being used.

Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
 libs/libltpnuma/tst_numa.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
index ef4c8e879..f6f577657 100644
--- a/libs/libltpnuma/tst_numa.c
+++ b/libs/libltpnuma/tst_numa.c
@@ -161,8 +161,11 @@ static int node_has_enough_memory(int node, size_t min_kb)
 
 	fclose(fp);
 
-	if (!mem_total || !mem_used) {
-		tst_res(TWARN, "Failed to parse '%s'", path);
+	tst_res(TINFO,"Node%i: mem_total:%ld kB, mem_used:%ld kB", node,
+	        mem_total, mem_used);
+
+	if (!mem_total) {
+		tst_res(TWARN, "Failed to parse '%s', MemTotal is 0", path);
 		return 0;
 	}
 
-- 
2.31.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Cyril Hrubis]

Hi!
>  
> -	if (!mem_total || !mem_used) {
> -		tst_res(TWARN, "Failed to parse '%s'", path);
> +	tst_res(TINFO,"Node%i: mem_total:%ld kB, mem_used:%ld kB", node,
> +	        mem_total, mem_used);
> +
> +	if (!mem_total) {
> +		tst_res(TWARN, "Failed to parse '%s', MemTotal is 0", path);
>  		return 0;

Why don't we use the return value from the sscanf instead?

What about:

diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
index ef4c8e879..7e7504cac 100644
--- a/libs/libltpnuma/tst_numa.c
+++ b/libs/libltpnuma/tst_numa.c
@@ -149,11 +149,19 @@ static int node_has_enough_memory(int node, size_t min_kb)
        while (fgets(buf, sizeof(buf), fp)) {
                long val;

-               if (sscanf(buf, "%*s %*i MemTotal: %li", &val) == 1)
+               if (sscanf(buf, "%*s %*i MemTotal: %li", &val) == 1) {
                        mem_total = val;
+               } else {
+                       tst_res(TWARN, "Faield to parse '%s' MemTotal", path);
+                       return 0;
+               }

-               if (sscanf(buf, "%*s %*i MemUsed: %li", &val) == 1)
+               if (sscanf(buf, "%*s %*i MemUsed: %li", &val) == 1) {
                        mem_used = val;
+               } else {
+                       tst_res(TWARN, "Faield to parse '%s' MemUsed", path);
+                       return 0;
+               }

                if (sscanf(buf, "%*s %*i FilePages: %li", &val) == 1)
                        file_pages = val;
@@ -161,10 +169,6 @@ static int node_has_enough_memory(int node, size_t min_kb)

        fclose(fp);

-       if (!mem_total || !mem_used) {
-               tst_res(TWARN, "Failed to parse '%s'", path);
-               return 0;
-       }

        mem_avail = mem_total - mem_used + (9 * file_pages)/10;


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Pengfei Xu]

Hi Cyril,

On 2023-05-09 at 13:59:01 +0200, Cyril Hrubis wrote:
> Hi!
> >  
> > -	if (!mem_total || !mem_used) {
> > -		tst_res(TWARN, "Failed to parse '%s'", path);
> > +	tst_res(TINFO,"Node%i: mem_total:%ld kB, mem_used:%ld kB", node,
> > +	        mem_total, mem_used);
> > +
> > +	if (!mem_total) {
> > +		tst_res(TWARN, "Failed to parse '%s', MemTotal is 0", path);
> >  		return 0;
> 
> Why don't we use the return value from the sscanf instead?
> 
> What about:
  I tried as follow:
# ./get_mempolicy01
tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
tst_numa.c:208: TINFO: Found 3 NUMA memory nodes
tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node0/meminfo' MemUsed:65220292
tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node1/meminfo' MemUsed:65167784
tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node2/meminfo' MemUsed:4194304
get_mempolicy01.c:149: TCONF: test requires at least one NUMA memory node
Node 0 and 1 could not be tested with follow patch installed, seems when
node2/meminfo "MemUsed: 0"，sscanf("...%li", &val) will return 0 if only 0 value
will be scaned, and the while loop will scan the char one by one and impact all
other node meminfo scan result, it's strange.

If used my patch, I didn't meet this issue.

Thanks!
BR.

> 
> diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
> index ef4c8e879..7e7504cac 100644
> --- a/libs/libltpnuma/tst_numa.c
> +++ b/libs/libltpnuma/tst_numa.c
> @@ -149,11 +149,19 @@ static int node_has_enough_memory(int node, size_t min_kb)
>         while (fgets(buf, sizeof(buf), fp)) {
>                 long val;
> 
> -               if (sscanf(buf, "%*s %*i MemTotal: %li", &val) == 1)
> +               if (sscanf(buf, "%*s %*i MemTotal: %li", &val) == 1) {
>                         mem_total = val;
> +               } else {
> +                       tst_res(TWARN, "Faield to parse '%s' MemTotal", path);
> +                       return 0;
> +               }
> 
> -               if (sscanf(buf, "%*s %*i MemUsed: %li", &val) == 1)
> +               if (sscanf(buf, "%*s %*i MemUsed: %li", &val) == 1) {
>                         mem_used = val;
> +               } else {
> +                       tst_res(TWARN, "Faield to parse '%s' MemUsed", path);
> +                       return 0;
> +               }
> 
>                 if (sscanf(buf, "%*s %*i FilePages: %li", &val) == 1)
>                         file_pages = val;
> @@ -161,10 +169,6 @@ static int node_has_enough_memory(int node, size_t min_kb)
> 
>         fclose(fp);
> 
> -       if (!mem_total || !mem_used) {
> -               tst_res(TWARN, "Failed to parse '%s'", path);
> -               return 0;
> -       }
> 
>         mem_avail = mem_total - mem_used + (9 * file_pages)/10;
> 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Cyril Hrubis]

Hi!
> # ./get_mempolicy01
> tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
> tst_numa.c:208: TINFO: Found 3 NUMA memory nodes
> tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node0/meminfo' MemUsed:65220292
> tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node1/meminfo' MemUsed:65167784
> tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node2/meminfo' MemUsed:4194304
> get_mempolicy01.c:149: TCONF: test requires at least one NUMA memory node
> Node 0 and 1 could not be tested with follow patch installed, seems when
> node2/meminfo "MemUsed: 0"，sscanf("...%li", &val) will return 0 if only 0 value
> will be scaned, and the while loop will scan the char one by one and impact all
> other node meminfo scan result, it's strange.
> 
> If used my patch, I didn't meet this issue.

Ah, right the file is multiline and we match only one of the lines per
iteration...

Then we need:

diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
index ef4c8e879..c3297013b 100644
--- a/libs/libltpnuma/tst_numa.c
+++ b/libs/libltpnuma/tst_numa.c
@@ -127,8 +127,8 @@ static int node_has_enough_memory(int node, size_t min_kb)
 {
        char path[1024];
        char buf[1024];
-       long mem_total = 0;
-       long mem_used = 0;
+       long mem_total = -1;
+       long mem_used = -1;
        long file_pages = 0;
        long mem_avail;

@@ -161,7 +161,7 @@ static int node_has_enough_memory(int node, size_t min_kb)

        fclose(fp);

-       if (!mem_total || !mem_used) {
+       if (mem_total == -1 || mem_used == -1) {
                tst_res(TWARN, "Failed to parse '%s'", path);
                return 0;
        }

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Pengfei Xu]

Hi Cyril,

On 2023-05-09 at 16:10:35 +0200, Cyril Hrubis wrote:
> Hi!
> > # ./get_mempolicy01
> > tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
> > tst_numa.c:208: TINFO: Found 3 NUMA memory nodes
> > tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node0/meminfo' MemUsed:65220292
> > tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node1/meminfo' MemUsed:65167784
> > tst_numa.c:162: TWARN: Faield to parse '/sys/devices/system/node/node2/meminfo' MemUsed:4194304
> > get_mempolicy01.c:149: TCONF: test requires at least one NUMA memory node
> > Node 0 and 1 could not be tested with follow patch installed, seems when
> > node2/meminfo "MemUsed: 0"，sscanf("...%li", &val) will return 0 if only 0 value
> > will be scaned, and the while loop will scan the char one by one and impact all
> > other node meminfo scan result, it's strange.
> > 
> > If used my patch, I didn't meet this issue.
> 
> Ah, right the file is multiline and we match only one of the lines per
> iteration...
> 
> Then we need:
> 
  Yes, below patch works well and as expected for all nodes memory check now.

  Below way is better and correct.

  Thanks!
  BR.

> diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
> index ef4c8e879..c3297013b 100644
> --- a/libs/libltpnuma/tst_numa.c
> +++ b/libs/libltpnuma/tst_numa.c
> @@ -127,8 +127,8 @@ static int node_has_enough_memory(int node, size_t min_kb)
>  {
>         char path[1024];
>         char buf[1024];
> -       long mem_total = 0;
> -       long mem_used = 0;
> +       long mem_total = -1;
> +       long mem_used = -1;
>         long file_pages = 0;
>         long mem_avail;
> 
> @@ -161,7 +161,7 @@ static int node_has_enough_memory(int node, size_t min_kb)
> 
>         fclose(fp);
> 
> -       if (!mem_total || !mem_used) {
> +       if (mem_total == -1 || mem_used == -1) {
>                 tst_res(TWARN, "Failed to parse '%s'", path);
>                 return 0;
>         }
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Cyril Hrubis]

Hi!
>   Yes, below patch works well and as expected for all nodes memory check now.
> 
>   Below way is better and correct.

I've pushed these changes along with your description, thanks!

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1 1/1] libs: libltpnuma: Fix one fake failure when CXL(Compute eXpress Link) node memory is not used

[Pengfei Xu]

On 2023-05-10 at 17:20:45 +0200, Cyril Hrubis wrote:
> Hi!
> >   Yes, below patch works well and as expected for all nodes memory check now.
> > 
> >   Below way is better and correct.
> 
> I've pushed these changes along with your description, thanks!

  LGTM, it's great! I'm glad that this issue was fixed.

  Thanks!
  BR.
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp