[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Li Wang]

These cases occasionally failed on RHEL platform:

numa02      2  TFAIL  :  ltpapicmd.c:200: Test #2: NUMA hit and othernode increase in node0 is less than expected
numa03      3  TFAIL  :  ltpapicmd.c:200: Test #3: NUMA interleave hit in node0 is less than expected
numa08      8  TFAIL  :  ltpapicmd.c:200: Test #8: NUMA interleave hit in node0 is less than expected

From git log (commit e439df0ea74231), it says "In RHEL collection of
istics take more time", and add sleep 2s in the case. I looked
into the detail and found that's reasonable, numastat growing slowly on
RHEL system, therefore tests failed with numastate update uncompleted.
(my sight is limited, if you have a different opinion, please comment)

Despite all that, sleeping 2 sec still working bad during my test.

In this patch, I replace 'sleep 2s' by detecting the numastat every 10ms,
if the threshold values keep changing in 2 sec, we consider that numastat
is on refreshing road, do loop again util 5 times.

The improved case running better than previous, but not perfect(100% PASS) everytime:

numa01 1 TINFO : The system contains 2 nodes:  0 1
numa01 1 TINFO : INIT: Numa tests will start now !!
numa01 1 TPASS : NUMA local node and memory affinity -TEST01 PASSED !!
numa01 2 TPASS : NUMA preferred node policy -TEST02 PASSED !!
numa01 3 TPASS : NUMA interleave policy -TEST03 PASSED !!
numa01 4 TPASS : NUMA phycpubind policy -TEST04 PASSED !!
numa01 5 TPASS : NUMA local node allocation -TEST05 PASSED !!
numa01 6 TPASS : NUMA interleave policy on shared memory -TEST06 PASSED !!
numa01 7 TPASS : NUMADEMO policies  -TEST07 PASSED !!
numa01 8 TPASS : NUMA MEMHOG policy -TEST08 PASSED !!
numa01 9 TPASS : NUMA policy on lib NUMA_NODE_SIZE API -TEST09 PASSED !!
numa01 10 TPASS : NUMA MIGRATEPAGES policy -TEST10 PASSED !!
numa01 11 TINFO : CLEAN: removing /tmp/ltp-ZJmMGHXeRw/tst_numa.1820

Test Environment: 15G RAM, 2 numa nodes, RHEL7.3GA, x86_64.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/numa/numa01.sh | 53 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/numa/numa01.sh b/testcases/kernel/numa/numa01.sh
index 9c5f49a..60dbae5 100755
--- a/testcases/kernel/numa/numa01.sh
+++ b/testcases/kernel/numa/numa01.sh
@@ -104,7 +104,35 @@ extract_numastat()
     return 0
 }
 
+# Function:     wait_for_update
+#
+# Input:        - $1 - node number.
+#               - $2 - numastat content.
+#
+# Description:  - waiting for numastat update
+#
+wait_for_update()
+{
+    NUMASTAT_PATH="/sys/devices/system/node/node$1/numastat"
+    local loop=0
 
+    while [ $loop -lt 5 ]; do
+        sum_value=0
+
+        for i in $(seq 200); do
+            det_value=$(grep $2 ${NUMASTAT_PATH} | cut -d ' ' -f 2)
+            sum_value=$((sum_value + det_value))
+
+            sync && tst_sleep 10ms
+        done
+
+        if [ $((sum_value/200)) -eq $det_value ]; then
+            return
+        fi
+
+        loop=$((loop+1))
+    done
+}
 
 # Function:     comparelog
 #
@@ -309,7 +337,10 @@ test02()
         extract_numastat other_node $other_node $col || return 1
         Prev_value=$RC
         numactl --cpunodebind=$node --preferred=$Preferred_node support_numa $ALLOC_1MB
-	sleep 2s	#In RHEL collection of statistics takes more time.
+
+        # In RHEL collection of statistics takes more time
+        wait_for_update $Preferred_node other_node
+
         numastat > $LTPTMP/numalog
         extract_numastat other_node $other_node $col || return 1
         Curr_value=$RC
@@ -363,7 +394,10 @@ test03()
     done
 
     numactl --interleave=all support_numa $ALLOC_1MB
-    sleep 2s        #In RHEL collection of statistics takes more time.
+
+    for node in `echo $nodes_list`; do
+        wait_for_update $node interleave_hit
+    done
 
     numastat > $LTPTMP/numalog
     COUNTER=1
@@ -514,7 +548,10 @@ test06()
     done
 
     numactl --length=1M --file /dev/shm/numa_shm --interleave=all --touch
-    sleep 2s        #In RHEL collection of statistics takes more time.
+
+    for node in `echo $nodes_list`; do
+        wait_for_update $node numa_hit
+    done
 
     numastat > $LTPTMP/numalog
     COUNTER=1
@@ -578,7 +615,10 @@ test07()
     done
 
     numademo -c ${msize}k > $LTPTMP/demolog
-    sleep 2s        #In RHEL collection of statistics takes more time.
+
+    for node in `echo $nodes_list`; do
+        wait_for_update $node interleave_hit
+    done
 
     numastat > $LTPTMP/numalog
     COUNTER=1
@@ -641,7 +681,10 @@ test08()
         COUNTER=$[$COUNTER+1]
     done
     numactl --interleave=all memhog 1MB
-    sleep 2s        #In RHEL collection of statistics takes more time.
+
+    for node in `echo $nodes_list`; do
+        wait_for_update $node interleave_hit
+    done
 
     numastat > $LTPTMP/numalog
     COUNTER=1
-- 
1.8.3.1

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Cyril Hrubis]

Hi!
> +wait_for_update()
> +{
> +    NUMASTAT_PATH="/sys/devices/system/node/node$1/numastat"
> +    local loop=0
>  
> +    while [ $loop -lt 5 ]; do
> +        sum_value=0
> +
> +        for i in $(seq 200); do
> +            det_value=$(grep $2 ${NUMASTAT_PATH} | cut -d ' ' -f 2)
> +            sum_value=$((sum_value + det_value))

I do not understand this part, here you are summing the accumated value
over and over. That is not making any sense.

> +            sync && tst_sleep 10ms

Why the sync here?

> +        done
> +
> +        if [ $((sum_value/200)) -eq $det_value ]; then

Here as well. It's higly unlikely that the value would be exactly equal
since the number is increased by other things the system does as well.

> +            return
> +        fi
> +
> +        loop=$((loop+1))
> +    done
> +}

This seems like a good idea generally but what would I do is something
as:

* Read the statistic with some small sleep in between as you do
* Exit once the increase is at least the expected value
* Give up if there was no increase in some well defined time or
  if it generally took too much time, i.e. something as:

  if the last increase of the number was more than second ago -> fail
  (this could be done easily by counting time since the last increase
   and resetting it if there was some)

  if the number was increased steadily for more than ten seconds but
  hasn't reached at least the expected value -> fail

The timing constants may need to be tuned, but AFAIC this is the best we
can do in this tests.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Li Wang]

On Wed, Nov 23, 2016 at 8:57 PM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
>> +wait_for_update()
>> +{
>> +    NUMASTAT_PATH="/sys/devices/system/node/node$1/numastat"
>> +    local loop=0
>>
>> +    while [ $loop -lt 5 ]; do
>> +        sum_value=0
>> +
>> +        for i in $(seq 200); do
>> +            det_value=$(grep $2 ${NUMASTAT_PATH} | cut -d ' ' -f 2)
>> +            sum_value=$((sum_value + det_value))
>
> I do not understand this part, here you are summing the accumated value
> over and over. That is not making any sense.

My purpose here is to get the average value of numastat in 2sec to
compare with $det_value which was detected at beginning. If they are
not equal, it means the statistic is on refreshing road, we should do
wait for updating. Otherwise, we consider that refresh work completed
and do return.

>
>> +            sync && tst_sleep 10ms
>
> Why the sync here?
>
>> +        done
>> +
>> +        if [ $((sum_value/200)) -eq $det_value ]; then
>
> Here as well. It's higly unlikely that the value would be exactly equal

No, at least for 'other_node' and 'interleave_hit' are two exceptions.

> since the number is increased by other things the system does as well.

I understand your worries, but for function wait_for_update(), I hope
only to take use of it for 'other_node/interleave_hit' detecting in
codes. From what I observe, these two values do not increase until
your give a rule for numa system.

# cat /sys/devices/system/node/node1/numastat
numa_hit 125820814
numa_miss 0
numa_foreign 0
interleave_hit 50180  <------
local_node 125779789
other_node 41025    <------

# sleep 300

# cat /sys/devices/system/node/node1/numastat
numa_hit 125821176
numa_miss 0
numa_foreign 0
interleave_hit 50180   <---- no changes here
local_node 125780151
other_node 41025      <----no changes


# numactl --cpunodebind=0 --preferred=1 ./support_numa 2

# cat /sys/devices/system/node/node1/numastat
numa_hit 125822847
numa_miss 0
numa_foreign 0
interleave_hit 50180   <----no changes
local_node 125781641
other_node 41206     <---- here increase


>> +    done
>> +}
>
> This seems like a good idea generally but what would I do is something
> as:
>
> * Read the statistic with some small sleep in between as you do
> * Exit once the increase is at least the expected value
> * Give up if there was no increase in some well defined time or
>   if it generally took too much time, i.e. something as:

Hmm,  I think our method is nearly the same, isn't it?

>
>   if the last increase of the number was more than second ago -> fail
>   (this could be done easily by counting time since the last increase
>    and resetting it if there was some)
>
>   if the number was increased steadily for more than ten seconds but
>   hasn't reached at least the expected value -> fail
>
> The timing constants may need to be tuned, but AFAIC this is the best we
> can do in this tests.
>
> --
> Cyril Hrubis
> chrubis@suse.cz

Thanks for you reviewing.

-- 
Regards,
Li Wang
Email: liwang@redhat.com

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Cyril Hrubis]

Hi!
I've been looking into the problem and found much better solution. We
can, instead of quering the whole system statistic, read statistic for a
single process.

What about we do:

1. Change support_numa.c to sleep in wait() instead of exitting when
   allocating memory
2. Run the numactl ... command on background with &
3. Then we can get statistic for a single process with numastat -p $pid
   Alternatively we can change the support_numa.c to allocate the memory
   with mmap and parse /proc/$pid/numa_maps to match the pointer address
   and figure out if the mapping(s) are distributed correctly
4. Then we can kill the $pid

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Li Wang]

On Thu, Dec 1, 2016 at 6:51 PM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> I've been looking into the problem and found much better solution. We
> can, instead of quering the whole system statistic, read statistic for a
> single process.

This method is great! But, I'm thinking about how can we apply to
test6, test7 and test8, because these functions do not involve
support_numa.c process in their test.

In test8:
   numactl --interleave=all memhog 1MB

It seems a little hard to hang up this memhog process.


Or, can we reseve previous method compatible with this? In other
words, two method exsit at the same time.

Li Wang


>
> What about we do:
>
> 1. Change support_numa.c to sleep in wait() instead of exitting when
>    allocating memory
> 2. Run the numactl ... command on background with &
> 3. Then we can get statistic for a single process with numastat -p $pid
>    Alternatively we can change the support_numa.c to allocate the memory
>    with mmap and parse /proc/$pid/numa_maps to match the pointer address
>    and figure out if the mapping(s) are distributed correctly
> 4. Then we can kill the $pid
>
> --
> Cyril Hrubis
> chrubis@suse.cz



-- 
Regards,
Li Wang
Email: liwang@redhat.com

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Cyril Hrubis]

Hi!
> This method is great! But, I'm thinking about how can we apply to
> test6, test7 and test8, because these functions do not involve
> support_numa.c process in their test.

I guess that we can add a dummy program to make wait for a signal in the test06.

Does something like this work?

numactl --length=1M --file /dev/shm/numa_shm --interleave=all --touch support_numa 3 &

I would be probably in favor of dropping the test07, since that does
more or less the same as test03.

> In test8:
>    numactl --interleave=all memhog 1MB
> 
> It seems a little hard to hang up this memhog process.

Just set large enough memhog repeat parameter with -r so that it does
not finish prematurely. Or even better we can just add a few lines of
code to the numa helper so that it can memset the memory in a loop just
like the memhog does.

https://github.com/numactl/numactl/blob/master/memhog.c

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/3] ltp/numa: waiting for numastat refresh

[Li Wang]

On Mon, Dec 5, 2016 at 5:28 PM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
>> This method is great! But, I'm thinking about how can we apply to
>> test6, test7 and test8, because these functions do not involve
>> support_numa.c process in their test.
>
> I guess that we can add a dummy program to make wait for a signal in the test06.
>
> Does something like this work?
>
> numactl --length=1M --file /dev/shm/numa_shm --interleave=all --touch support_numa 3 &

Sounds good, we can just add raise(SIGSTOP) in support_numa.c to wait a signal.

>
> I would be probably in favor of dropping the test07, since that does
> more or less the same as test03.

Ok, agree to remove.

>
>> In test8:
>>    numactl --interleave=all memhog 1MB
>>
>> It seems a little hard to hang up this memhog process.
>
> Just set large enough memhog repeat parameter with -r so that it does
> not finish prematurely. Or even better we can just add a few lines of
> code to the numa helper so that it can memset the memory in a loop just
> like the memhog does.
>
> https://github.com/numactl/numactl/blob/master/memhog.c

Great! Let me have a look and then choose a way.

Thanks,
Li Wang