awslabs / damo

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

Is there a way to set `--numa_node` for each scheme in `json` file? #65

Closed honggyukim closed 1 month ago

honggyukim commented 1 year ago

Hi SeongJae,

I would like to create two different schemes, but also want to limit the monitoring scope separately.

For example, let's say there are action A and B. Then I want to apply action A to NUMA node 0 and also want to apply action B to NUMA node 1.

So is there a way to set two different --numa_node options for each scheme in .json file? (I saw it's possible to provide two schemes only with .json file.)

Thanks.

sj-aws commented 1 year ago

For such use cases, DAMON sysfs interface supports multiple kdamonds. That is, you could create 2 kdamonds, each for {NUMA node 0/action A scheme} and {NUMA node 1/actionB scheme} respectively. Also, we plan to support multiple contexts on single scheme. If that's done, you could create two contexts on single kdamond, each works for the NUMA node 0 and 1, respectively.

However, damo is not supporting the multiple kdamonds usecase at the moment. It has been TODO for a long time. I will start work on it soon.

Also, a better approach here would be adding yet another type of DAMOS filter that works based on address range. That is, the feature will allow you to specify to which address ranges the scheme's action should be applied. Using this, you could monitor both NUMA node 0 and 1 while having two schemes having different address range based filter. Such filter is not supported by DAMON at the moment. As I found a use case that it could be useful, I will soon start working on it, too.

The implementations would take some time, though. Please bear in mind with me.

If you don't think some of the two works wouldn't work for you, or you have some suggestions, please let me know.

honggyukim commented 1 year ago

However, damo is not supporting the multiple kdamonds usecase at the moment. It has been TODO for a long time. I will start work on it soon.

Thanks. I understand why it's implemented that way. I'm fine with the current one because I won't too much rely on damo now.

Also, a better approach here would be adding yet another type of DAMOS filter that works based on address range. That is, the feature will allow you to specify to which address ranges the scheme's action should be applied. Using this, you could monitor both NUMA node 0 and 1 while having two schemes having different address range based filter.

We also thought about adding another filther called numa and it can simply be implemented because it looks node ID, nid, can easily be accessed from the given folio or page, then it can be used at __damos_pa_filter_out. So we can try to add the numa filter if this is not an incorrect approach.

Thanks for the explanation again.

sj-aws commented 1 year ago

Thanks. I understand why it's implemented that way. I'm fine with the current one because I won't too much rely on damo now.

That's a relief, but also sad in same time that you don't rely on damo :crying_cat_face:

We also thought about adding another filther called numa

I was also thinking about it first. However, I started thinking address based filter could also work, because normally each numa node has contiguous address space. Also, it would be more flexible. I think we can implement numa type DAMOS filter on damo, though. That is, implement only address ranges type DAMOS filter in DAMON (in kernel), but implement both address ranges type and numa type DAMOS filter on damo. The numa type filter on damo will translate numa id to address range and use address ranges type filter of DAMON internally.

honggyukim commented 1 year ago

That's a relief, but also sad in same time that you don't rely on damo :crying_cat_face:

Having damo is super helpful when testing our features, but I think that the usage can be simpler when it's implemented as a static kernel module, then users just turn the switch on from /sys/module/damon_XXX.

I was also thinking about it first. However, I started thinking address based filter could also work, because normally each numa node has contiguous address space. Also, it would be more flexible.

I agree with that.

I think we can implement numa type DAMOS filter on damo, though. That is, implement only address ranges type DAMOS filter in DAMON (in kernel), but implement both address ranges type and numa type DAMOS filter on damo. The numa type filter on damo will translate numa id to address range and use address ranges type filter of DAMON internally.

That totally makes sense. But I will need to do the same damo implementation in my static kernel module again just like the case finding the largest region in both damo and DAMON in kernel as follows.

sj-aws commented 1 year ago

The address based DAMOS filter support DAMON implementation has posted as an RFC patchset: https://lore.kernel.org/damon/20230728203444.70703-1-sj@kernel.org/

honggyukim commented 1 year ago

I'm sorry for the late response but I really appreciate your help. I can't look into the details now due to our internal priority but I will take and use the feature when I will be available for testing.

honggyukim commented 1 year ago

The address based DAMOS filter support DAMON implementation has posted as an RFC patchset: https://lore.kernel.org/damon/20230728203444.70703-1-sj@kernel.org/

I just tried to apply your patches to my kernel repo, but it shows some conflicts. It'd be useful if the patches were created with --base option in git format-patch so that b4 command can inform what the base hash is.

$ b4 am https://lore.kernel.org/damon/20230728203444.70703-1-sj@kernel.org/
Grabbing thread from lore.kernel.org/damon/20230728203444.70703-1-sj%40kernel.org/t.mbox.gz
Analyzing 14 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH RFC 1/13] mm/damon/core: introduce address range type damos filter
    ✓ Signed: DKIM/kernel.org
  ✓ [PATCH RFC 2/13] mm/damon/sysfs-schemes: support address range type DAMOS filter
    ✓ Signed: DKIM/kernel.org
  ERROR: missing [3/13]!
  ✓ [PATCH RFC 4/13] selftests/damon/sysfs: test address range damos filter
    ✓ Signed: DKIM/kernel.org
...
  ✓ [PATCH RFC 13/13] Docs/admin-guide/mm/damon/usage: update for DAMON monitoring target type DAMOS filter
    ✓ Signed: DKIM/kernel.org
  ---
  NOTE: install patatt for end-to-end signature verification
---
Total patches: 12
---
WARNING: Thread incomplete!
Cover: ./20230728_sj_extedn_damos_filters_for_address_ranges_and.cover
 Link: https://lore.kernel.org/r/20230728203444.70703-1-sj@kernel.org
 Base: not specified
       git am ./20230728_sj_extedn_damos_filters_for_address_ranges_and.mbx

It currently shows Base: not specified.

sj-aws commented 1 year ago

Hi Honggyu,

I can't look into the details now due to our internal priority but I will take and use the feature when I will be available for testing.

no problem, no rush. Please do on your pace :)

It'd be useful if the patches were created with --base option in git format-patch so that b4 command can inform what the base hash is.

Good point, and sorry for the issue. DAMON patches are always based on mm-unstable, and you can get those from latest DAMON development tree[1]. Anyway, adding --base option would be the right thing to do, so I updated my script[2].

[1] https://git.kernel.org/sj/h/damon/next [2] https://git.kernel.org/pub/scm/linux/kernel/git/sj/damon-hack.git/commit/?id=804633fd3bd12dc1cfdb4eb39e97caa06b1a390a****

honggyukim commented 1 year ago

Hi SeongJae,

Good point, and sorry for the issue. DAMON patches are always based on mm-unstable, and you can get those from latest DAMON development tree[1]. Anyway, adding --base option would be the right thing to do, so I updated my script[2].

I saw that the new patches include base-commit in https://lore.kernel.org/linux-mm/20230909033711.55794-1-sj@kernel.org/T/#t. Thanks for doing that!

sj-aws commented 1 month ago

After 2024-09-05, we will[1] use damonitor repo[2] as the only main repo for GitHub. To continue discussion of this issue on the repo, I just created a new issue[3] there. Please use the new issue for followup discussions.

Closing this issue in favor of the new one on the damonitor repo. Please ask me to open this again if you cannot continue the discussion from the new issue.

[1] https://lore.kernel.org/20240813232158.83903-1-sj@kernel.org [2] https://github.com/damonitor/damo [3] https://github.com/damonitor/damo/issues/5