awslabs / damo

DAMON user-space tool
https://damonitor.github.io/
GNU General Public License v2.0
155 stars 26 forks source link

It seems "damo schemes" doesn't use physical address option (--ops paddr) #54

Closed honggyukim closed 11 months ago

honggyukim commented 1 year ago

Hi SeongJae,

I have executed the damo schemes with --ops paddr as follows.

$ sudo ./damo schemes --damos_access_rate 0 0 --damos_sz_region 4K max \
               --damos_age 1s max --ops paddr --damos_action pageout \
               "./masim/masim ./masim/configs/stairs.cfg"

However, it didn't go through the damon_pa_apply_scheme and instead it goes to damon_va_apply_scheme in the kernel implementation.

I have checked the operations file in sysfs during the above masim execution, but it shows vaddr.

# cat /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/operations
vaddr

I'm just wondering if my usage for --ops paddr is incorrect.

Thanks.

sj-aws commented 1 year ago

Thank you for this report, I will take a look soon.

honggyukim commented 1 year ago

I found that the args.ops is set to vaddr whatever users add an option to --ops.

Rather than doing this, it looks like we better change the default option for --ops from paddr to vaddr then remove the line as follows.

diff --git a/_damon_args.py b/_damon_args.py
index c137b78..ec22445 100644
--- a/_damon_args.py
+++ b/_damon_args.py
@@ -180,7 +180,6 @@ def deduce_target_update_args(args):
     elif target_type == target_type_pid:
         pid = int(args.deducible_target)
     args.target_pid = pid
-    args.ops = 'vaddr'
     if args.regions:
         args.ops = 'fvaddr'

@@ -292,7 +291,7 @@ def set_argparser(parser, add_record_options):
         parser = argparse.ArgumentParser()
     set_monitoring_argparser(parser)
     parser.add_argument('--ops', choices=['vaddr', 'paddr', 'fvaddr'],
-            default='paddr',
+            default='vaddr',
             help='monitoring operations set')
     parser.add_argument('--target_pid', type=int, help='target pid')
     set_damos_argparser(parser)

It works fine with vaddr mode, but it fails damo execution when --ops paddr is given as follows.

$ sudo ./damo schemes --damos_access_rate 0 0 --damos_sz_region 4K max --damos_age 1s max \
                      --ops paddr --damos_action pageout \
                      "./masim/masim ./masim/configs/default"
Press Ctrl+C to stop
Traceback (most recent call last):
  File "/home/honggyu/work/damo/./damo", line 103, in <module>
    main()
  File "/home/honggyu/work/damo/./damo", line 100, in main
    subcmd.execute(args)
  File "/home/honggyu/work/damo/_damo_subcmds.py", line 30, in execute
    self.module.main(args)
  File "/home/honggyu/work/damo/damo_schemes.py", line 53, in main
    os.waitpid(kdamonds[0].contexts[0].targets[0].pid, 0)
TypeError: 'NoneType' object cannot be interpreted as an integer

I will investigate more why it fails.

sj-aws commented 1 year ago

Thank you for further investigating this issue.

--ops option is for explicitly setting the DAMON operations set, while the command argument is for asking damo to do the command and set the virtual address space of the resulting process as the monitoring target. In the latter case, because the target is a virtual address space, vaddr operations set is used.

What option should be ignored when the two options are set together? I didn't prepare a good answer yet. However, I also couldn't figure out what the user really wants to do with the command.

If there is no clear expectation and intention for the combination of the inputs, I'd say it might make some sense to keep current behavior (ignore --ops when it's given together with the command argument), or simply provide error message asking more clear request.

May I ask your opinion, Honggyu?

honggyukim commented 1 year ago

Thanks very much for your answer.

Do you mean that "./masim/masim ./masim/configs/default" is the command argument and this part forces damo to ignore --ops paddr?

If so, does it mean that paddr mode is for system wide monitoring and collect all the memory accesses across all the processes in the system? If this is true, then I would like to know if there is a way to collect memory accesses only from a given pid.

My use case is that I need to monitor memory accesses only from a specific NUMA node. Could you please correct me if the following usage make sense?

# run a target workload
$ ./masim/masim ./masim/configs/stairs.cfg &

# collect the workload's pid
$ export PID=$!

# run damo in physical address mode in numa node 0 for the given pid
$ sudo ./damo schemes --damos_access_rate 0 0 --damos_sz_region 4K max \
               --damos_age 1s max --damos_action pageout \
               --ops paddr --numa_node 0 \
               $PID

If this usage providing $PID is same as giving the command argument, then would this be reasonable?

$ sudo ./damo schemes --damos_access_rate 0 0 --damos_sz_region 4K max \
               --damos_age 1s max --damos_action pageout \
               --ops paddr --numa_node 0

If this is the correct usage, then I'm wondering how I can collect memory accesses only from some specific workload target.

Could you please correct me if I misunderstood something?

sj-aws commented 1 year ago

Hi Honggyu,

The usage of --ops is not yet documented, and hence still not stable[1] and could be changed in future. Nevertheless, let me explain the intended usage of it at the moment.

You can set the monitoring operations set and the monitoring targets for DAMON. Currently, three monitoring operations set, namely paddr, vaddr, and fcaddr are supported. For each of the operations set, different types of targets can be set. For virtual address spaces monitoring operations set (vaddr and fvaddr), a list of pids and virtual address ranges of the virtual address spaces could be specified as targets. For physical address space monitoring operations set (paddr), physical address ranges could be specified as targets.

damo schemes command arguments allow two ways to specify the operations set and monitoring targets: explicit and implicit. In explicit way, you should specify the operations set via --ops option, and monitoring target address ranges via --regions option. If --ops is virtual address spaces supporting set, you can further set the monitoring target pids via --target_pid, or the positional argument.

In implicit way, you could put a command as the positional argument. It means damo should execute the command, set --ops as vaddr if --regions is not set, else fvaddr, and set the pid of the process spawned by the command as the monitoring target pid.

Do you mean that "./masim/masim ./masim/configs/default" is the command argument and this part forces damo to ignore --ops paddr?

As mentioned above, it doesn't ignore --ops always, but in some case, specifically when the positional argument is a command and --ops is paddr, it does.

If so, does it mean that paddr mode is for system wide monitoring and collect all the memory accesses across all the processes in the system?

paddr does the system wide monitoring by default, but it respects --regions option.

I would like to know if there is a way to collect memory accesses only from a given pid.

No, DAMON is not supporting that at the moment, and hence same to damo. We have a plan to extend DAMON for monitoring accesses from specific CPUs. I think it could further extended for processes. It would take some time, and currently not very highly prioritized.

If this usage providing $PID is same as giving the command argument, then would this be reasonable?

Yes, that looks reasonable to me. It means the user wants damo to do the proactive reclamation for only node 0.

I'm wondering how I can collect memory accesses only from some specific workload target.

As mentioned above, DAMON doesn't support that at the moment.

Maybe another approach for your usecase is, doing the proactive reclamation based on the target workloads' virtual address space, but limit the pageout action to specific NUMA node. DAMON doesn't support that at the moment, but I think it could be implemented by extending DAMOS filters[2] feature. And I think that could be easier to implement compared to extending DAMON to monitor accesses from specific workloads.

Please let me know if it makes sense to you, and if so how important it could be for you, so that I could adjust priorities of my todo items.

[1] https://github.com/awslabs/damo#why-some-subcommands-are-not-documented-on-usagemd-file [2] https://www.kernel.org/doc/html/next/mm/damon/design.html#filters

honggyukim commented 1 year ago

Hi SeongJae,

The usage of --ops is not yet documented, and hence still not stable[1] and could be changed in future. Nevertheless, let me explain the intended usage of it at the moment.

I found that the damo doesn't accept command arguments such as "./masim ..." when --ops paddr is given. I have tested it again without command arguments as follows.

$ sudo ./damo schemes --damos_access_rate 0 0 --damos_sz_region 4K max \
               --damos_age 1s max --damos_action pageout \
               --ops paddr --numa_node 0

Then I ran masim separately, then it worked as expected so I think this usage should be fine.

damo schemes command arguments allow two ways to specify the operations set and monitoring targets: explicit and implicit. In explicit way, you should specify the operations set via --ops option, and monitoring target address ranges via --regions option.

Ack. I found that --numa_node 0 created 5 region ranges as follows.

$ cat /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/targets/0/regions/nr_regions
5

Does it mean that --numa_node is another way of specifying --regions but automatically finds the regions based on numa info?

Do you mean that "./masim/masim ./masim/configs/default" is the command argument and this part forces damo to ignore --ops paddr?

As mentioned above, it doesn't ignore --ops always, but in some case, specifically when the positional argument is a command and --ops is paddr, it does.

I think it would be better if damo to be changed is to complain an error message when --ops has to be ignored rather than ignoring, which looks confusing to users.

If so, does it mean that paddr mode is for system wide monitoring and collect all the memory accesses across all the processes in the system?

paddr does the system wide monitoring by default, but it respects --regions option.

Thanks for the clarification.

I would like to know if there is a way to collect memory accesses only from a given pid.

No, DAMON is not supporting that at the moment, and hence same to damo. We have a plan to extend DAMON for monitoring accesses from specific CPUs. I think it could further extended for processes. It would take some time, and currently not very highly prioritized.

I'm fine not having this usage because I saw that the target process can be executed separately, but it can still be monitored by DAMON properly.

If this usage providing $PID is same as giving the command argument, then would this be reasonable?

Yes, that looks reasonable to me. It means the user wants damo to do the proactive reclamation for only node 0.

This use case fits my purpose.

Maybe another approach for your usecase is, doing the proactive reclamation based on the target workloads' virtual address space, but limit the pageout action to specific NUMA node. DAMON doesn't support that at the moment, but I think it could be implemented by extending DAMOS filters[2] feature. And I think that could be easier to implement compared to extending DAMON to monitor accesses from specific workloads.

Thanks. I will think about it more.

Please let me know if it makes sense to you, and if so how important it could be for you, so that I could adjust priorities of my todo items.

Thanks very much for your continuous support. I think I don't have to monitor a specific PID in paddr mode now as I found that damo schemes works fine system widely in paddr mode. I think ask you again if I have a specific use case later for that.

sj-aws commented 1 year ago

Does it mean that --numa_node is another way of specifying --regions but automatically finds the regions based on numa info?

You're correct.

I think it would be better if damo to be changed is to complain an error message when --ops has to be ignored rather than ignoring, which looks confusing to users.

Agreed. I will make the change soon, unless others are faster.

Thanks very much for your continuous support. I think I don't have to monitor a specific PID in paddr mode now as I found that damo schemes works fine system widely in paddr mode. I think ask you again if I have a specific use case later for that.

My big pleasure, and thank you for giving me this interesting questions that enlightens me. Please feel free to ask any more questions or helps :)

sj-aws commented 1 year ago

I added https://github.com/awslabs/damo/issues/60 as a followup of this discussion.

sj-aws commented 11 months ago

I guess all works for this issue are done, if I'm not missing something? Anything still needed?

honggyukim commented 11 months ago

Hi SeongJae, sorry for the late response again. I missed adding a comment for this. I think this issue is fixed so I will close it. Thanks very much for your help!

sj-aws commented 11 months ago

No problem. Please feel free to open any ticket if needed later :)