Avoid double strip() calls for getting memory type.

Signed-off-by: Alexander Pantyukhin <apantykhin@gmail.com>
---
 tools/perf/scripts/python/mem-phys-addr.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/scripts/python/mem-phys-addr.py b/tools/perf/scripts/python/mem-phys-addr.py
index 1f332e72b9b0..f267d8382eda 100644
--- a/tools/perf/scripts/python/mem-phys-addr.py
+++ b/tools/perf/scripts/python/mem-phys-addr.py
@@ -8,7 +8,6 @@ from __future__ import print_function
 
 import os
 import sys
-import struct
 import re
 import bisect
 import collections
@@ -30,12 +29,14 @@ event_name = None
 def parse_iomem():
 	global f
 	f = open('/proc/iomem', 'r')
-	for i, j in enumerate(f):
+	for _, j in enumerate(f):
 		m = re.split('-|:',j,2)
-		if m[2].strip() == 'System RAM':
+		memory_type = m[2].strip()
+
+		if memory_type == 'System RAM':
 			system_ram.append(int(m[0], 16))
 			system_ram.append(int(m[1], 16))
-		if m[2].strip() == 'Persistent Memory':
+		elif memory_type == 'Persistent Memory':
 			pmem.append(int(m[0], 16))
 			pmem.append(int(m[1], 16))
 
@@ -75,6 +76,7 @@ def is_persistent_mem(phys_addr):
 def find_memory_type(phys_addr):
 	if phys_addr == 0:
 		return "N/A"
+
 	if is_system_ram(phys_addr):
 		return "System RAM"
 
-- 
2.25.1

Hi!
There are some things which I would like to discuss there.

1) I found that if there is for example the following system_ram:
system_ram = [int('00000000', 16) , int('0000ffff', 16)]

and the func

def is_system_ram(phys_addr):
    #/proc/iomem is sorted
    position = bisect.bisect(system_ram, phys_addr)
    if position % 2 == 0:
    return False
    return True

The value:
is_system_ram(int('0000ffff', 16)) is False.
It seems to be a mistake. This value is the end of the range and it
should be a True.
(There is another check on the 88 line: if int(m[0], 16) <= phys_addr
<= int(m[1], 16)
where both ranges are included in the check.)

2) The script reads the whole file when parse_iomem()
and uses this information for pmem and system_ram. What if to store
whole info about addresses
for all memory types?
In This case it's possible to store the data structure like the
following example:

/proc/iomem
00000000-0000ffff : reserved
00010000-0009f3ff : System RAM

Result: [(00000000, 'reserved'), (0000ffff, 'reserved'), (00010000,
'System RAM'), (0009f3ff, 'System RAM')....]

This case the searching would be O(log(n)) for all types not just for
"System RAM" and "Persistent Memory".

But the behavior of the script would be changed: the current version
re-read the file and scan the fresh data
when neither "System RAM" nor "Persistent Memory". But the suggested
fix requires to call parse_iomem() when
fresh data is required.

If these changes are approved, I would like to fix it.

Best, Alex.

пн, 9 янв. 2023 г. в 13:33, Alexander Pantyukhin <apantykhin@gmail.com>:
>
> Avoid double strip() calls for getting memory type.
>
> Signed-off-by: Alexander Pantyukhin <apantykhin@gmail.com>
> ---
>  tools/perf/scripts/python/mem-phys-addr.py | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/scripts/python/mem-phys-addr.py b/tools/perf/scripts/python/mem-phys-addr.py
> index 1f332e72b9b0..f267d8382eda 100644
> --- a/tools/perf/scripts/python/mem-phys-addr.py
> +++ b/tools/perf/scripts/python/mem-phys-addr.py
> @@ -8,7 +8,6 @@ from __future__ import print_function
>
>  import os
>  import sys
> -import struct
>  import re
>  import bisect
>  import collections
> @@ -30,12 +29,14 @@ event_name = None
>  def parse_iomem():
>         global f
>         f = open('/proc/iomem', 'r')
> -       for i, j in enumerate(f):
> +       for _, j in enumerate(f):
>                 m = re.split('-|:',j,2)
> -               if m[2].strip() == 'System RAM':
> +               memory_type = m[2].strip()
> +
> +               if memory_type == 'System RAM':
>                         system_ram.append(int(m[0], 16))
>                         system_ram.append(int(m[1], 16))
> -               if m[2].strip() == 'Persistent Memory':
> +               elif memory_type == 'Persistent Memory':
>                         pmem.append(int(m[0], 16))
>                         pmem.append(int(m[1], 16))
>
> @@ -75,6 +76,7 @@ def is_persistent_mem(phys_addr):
>  def find_memory_type(phys_addr):
>         if phys_addr == 0:
>                 return "N/A"
> +
>         if is_system_ram(phys_addr):
>                 return "System RAM"
>
> --
> 2.25.1
>