awslabs / damo

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

Support a single config file that describes all the options #76

Closed honggyukim closed 9 months ago

honggyukim commented 9 months ago

Hi SeongJae,

We've been heavily testing DAMOS based on our custom actions and found a possible use case so I've created this issue.

We currently use damo schemes -c action.json with more custom options such as --sample, --aggr, and --monitoring_nr_regions_range together. We keep each json config file for each execution, but extra options must be maintained together.

I feel that it will be really useful if there is a single config file that describes all the damo options including damo options and DAMOS config options together.

I think one single json file can be provided for this purpose so it'd be helpful if you could consider this use case. Thanks!

sj-aws commented 9 months ago

Hi Honggyu,

Thank you again for this nice usage sharing and new feature suggestion! I agree that could be very useful. Actually, the feature is already available in damo, though. However, as usual, I was too lazy to document and advertise the feature. Sorry again about it.

There are a group of damo commands that controls DAMON. The group includes start, stop, tune, record, and schemes. Except stop, the commands need to start or tune DAMON in user-requested way, so shares a set of command line options for that purpose. The options you mentioned, including --samepl and --aggr are also a part of it. The set of options also contains --kdamonds option. You can put a json string or file that contains the DAMON parameters specification. Using it, you can specify the intervals with schemes at once. There is a command for converting your command line options to the json string, namely fmt_json. Try damo fmt_json --damos_action stat --damos_filter anon matching for example.

Note that the DAMON-control commands except stop commonly provides one positional argument, called <deducible target>. That could be

Hence, you could also put the json format total option via the one positional argument.

Again, this confusion is due to the lack of the document. I didn't make it basically due to my laziness, but also because I wasn't sure if the feature is somewhat really useful and stable. You made me believe it could be used in real. Making it stable is what I should do, and I will do. Hopefully the official documentation of damo will contain it in near future.

sj-aws commented 9 months ago

I just found the option was broken, and fixed by https://github.com/awslabs/damo/commit/b795a5e09cae2ba6e9cd39f3b22df351e0e9e79f.

With the commit, I confirmed what I explained above works as below:

$ sudo ./damo fmt_json
{
    "kdamonds": [
        {
            "state": null,
            "pid": null,
            "contexts": [
                {
                    "intervals": {
                        "sample_us": "5 ms",
                        "aggr_us": "100 ms",
                        "ops_update_us": "1 s"
                    },
                    "nr_regions": {
                        "min": "10",
                        "max": "1,000"
                    },
                    "ops": "paddr",
                    "targets": [
                        {
                            "pid": null,
                            "regions": [
                                {
                                    "start": "4,294,967,296",
                                    "end": "136,299,151,359"
                                }
                            ]
                        }
                    ],
                    "schemes": []
                }
            ]
        }
    ]
}
$ sudo ./damo fmt_json > default.json
$ sudo ./damo start default.json
$ sudo ./damo status
kdamond 0
    state: on, pid: 3053
    context 0
        ops: paddr
        intervals: sample 5 ms, aggr 100 ms, update 1 s
        nr_regions: [10, 1,000]
        target 0
            pid: 0
            region [4,294,967,296, 136,299,151,359) (122.938 GiB)
honggyukim commented 9 months ago

Hi SeongJae,

Thanks very much for the fix! It looks great.

I'm just wondering if it allows us to run more than 2 kdamonds running.

sj-aws commented 9 months ago

Hi Honggyu,

I'm just wondering if it allows us to run more than 2 kdamonds running.

Yes, it is assumed to support the use case. Nevertheless, the use case is not in its test case (tests/), so I'm not very confident if it still works. Please try and let me know if it fails. Sharing your success would also very helpful for damo development.

It is also further intended to support two or more contexts in future, but it is not supported at the moment.

honggyukim commented 9 months ago

Hi SeongJae,

I've tested it but it doesn't work as follows.

$ ./damo start two_kdamonds.json
could not turn on damon (cannot apply kdamonds from args (currently only <=one kdamond is supported))

However, if the check routine is removed, then I see that it works fine.

diff --git a/_damon_sysfs.py b/_damon_sysfs.py
index 47c91af..b9a994f 100644
--- a/_damon_sysfs.py
+++ b/_damon_sysfs.py
@@ -305,8 +305,6 @@ def ensure_dirs_populated_for(kdamonds):
         exit(1)

 def stage_kdamonds(kdamonds):
-    if len(kdamonds) > 1:
-        return 'currently only <=one kdamond is supported'
     if len(kdamonds) == 1 and len(kdamonds[0].contexts) > 1:
         return 'currently only <=one damon_ctx is supported'
     ensure_dirs_populated_for(kdamonds)
$ ./damo start two_kdamonds.json

$ tree -L 2 /sys/kernel/mm/damon/admin/kdamonds
/sys/kernel/mm/damon/admin/kdamonds
|-- 0
|   |-- contexts
|   |-- pid
|   `-- state
|-- 1
|   |-- contexts
|   |-- pid
|   `-- state
`-- nr_kdamonds

$ ps aux | grep kdamond
root      3138  0.0  0.0      0     0 ?        I    04:04   0:00 [kdamond.0]
root      3139  0.0  0.0      0     0 ?        I    04:04   0:00 [kdamond.1]

I think I can create two kdamonds for my purpose instead of using address range filter. Thanks!

sj-aws commented 9 months ago

Thank you for sharing the test results, Honggyu! I was thinking I removed the check routine, but obviously I was wrong. Please feel free to PR the check routine removal!

honggyukim commented 9 months ago

Thank you. Creating two kdamonds can be really useful. I will send the PR within a few days.

honggyukim commented 9 months ago

Hi SeongJae,

I've just create a PR at #78. Thanks!