Make --damos_filter easier to use regards of meaning of matching/nomatching

[sj-aws]

To continue the discussion of this topic from skhynix/hmsdk#3

[sjp38]

Summary of previous discussion

My immediate idea was making matching argument of --damos_filter to receive 'in' and 'out' in addition to 'matching' and 'nomatching'.

@honggyukim suggested adding --damos_filter_in and --damos_filter_out options (skhynix/hmsdk#3 (comment)). To me, both options sounds similar.

I don't like both

But now I remember that I didn't want to have "in" filter concept at all (https://lore.kernel.org/damon/[email protected]/). Biggest concern to me is that it is ambiguous about the relation between multiple inclusion filters. To quote the part from the previous discussion:

if we have a filter that
includes only pages of type A, but if there could be yet another filter that
includes only pages of type B, would the consequence is the action being
applied to pages of type A and B? Or, type A or type B?

Note that current DAMON implementation will work as "and". I tried to interpret this in my brain, and now I conclude again that this is confusing to me. As a maintainer, I don't want to have it for now (who knows, I might change my mind later).

My suggestion

Our purpose is making the option less confusing. For the purpose, I think --damos_filter_out option with matching argument can work. --damos_filter_in together will only make it more confusing, imho.

E.g.,

• --damos_action migrate_cold --damos_filter_out matching young: demote pages except young pages
• --damos_action migrate_hot --damos_filter_out nomatching young: promote pages except non-young pages
• --damos_filter_out matching type_A --damos_filter_out matching type_B: exclude pages of type A and B.

I understand having two negative terms can make it difficult to understand. Maybe we can think about old filter type (not on DAMON, but on damo) if it helps. E.g.,

• --damos_action migrate_cold --damos_filter_out matching young: demote pages except young (accessed) pages
• --damos_action migrate_hot --damos_filter_out matching old: promote pages except old (un-accessed) pages

We can also make "matching" argument as optional, with default value "matching". Then the example becomes

• --damos_action migrate_cold --damos_filter_out young: demote pages except young (accessed) pages
• --damos_action migrate_hot --damos_filter_out old: promote pages except old (un-accessed) pages

@honggyukim What do you think?

[honggyukim]

Hi SeongJae,

Thanks very much for the summary. Please see my inline comments.

But now I remember that I didn't want to have "in" filter concept at all (https://lore.kernel.org/damon/[email protected]/). Biggest concern to me is that it is ambiguous about the relation between multiple inclusion filters.

Make sense. I now understand having multiple inclusion filters make things confusing in DAMOS filter concept.

Note that current DAMON implementation will work as "and". I tried to interpret this in my brain, and now I conclude again that this is confusing to me. As a maintainer, I don't want to have it for now (who knows, I might change my mind later).

I agree now.

My suggestion

Our purpose is making the option less confusing. For the purpose, I think --damos_filter_out option with matching argument can work. --damos_filter_in together will only make it more confusing, imho.

I also think --damos_filter_out without --damos_filter_in looks good, but if we're okay to introduce such a pseudo option, then we might think about making it --damos_exclude as a possible candidate.

In addition, I think we can use python's not operator concept here rather than matching and nomatching. We can make matching by default, then check if there is not, then make it nomatching. This can be a possible option because we won't introduce a new filter type named not. (maybe)

E.g.,

• --damos_action migrate_cold --damos_filter_out matching young: demote pages except young pages
• --damos_action migrate_hot --damos_filter_out nomatching young: promote pages except non-young pages

These can be as follows.

• --damos_action migrate_cold --damos_filter_out young: demote pages except young pages
• --damos_action migrate_hot --damos_filter_out not young: promote pages except not young pages

• --damos_filter_out matching type_A --damos_filter_out matching type_B: exclude pages of type A and B.

This can be much simpler with --damos_filter_out type_A --damos_filter_out type_B.

I understand having two negative terms can make it difficult to understand. Maybe we can think about old filter type (not on DAMON, but on damo) if it helps. E.g.,

Making matching by default and using not only in nomatching will make it much simpler. If you're okay with this approach, then I think old is not needed.

We can also make "matching" argument as optional, with default value "matching". Then the example becomes

• --damos_action migrate_cold --damos_filter_out young: demote pages except young (accessed) pages
• --damos_action migrate_hot --damos_filter_out old: promote pages except old (un-accessed) pages

@honggyukim What do you think?

I didn't read until this part and made the comment above. But you thought it in the similar ways. :)

[sjp38]

Thanks for the input, Honggyu! It seems --damos_filter_out with matching by default is the path forward. I'll implement it unless someone raises a different opinion.

[honggyukim]

Thanks for the quick response.

In addition, I think we can use python's not operator concept here rather than matching and nomatching. We can make matching by default, then check if there is not, then make it nomatching. This can be a possible option because we won't introduce a new filter type named not. (maybe)

I also suggested using optional not instead of nomatching. What do you think about it?

[sjp38]

To me, using nomatching or not looks not very critical, but not based usage looks bit better. I'll update the interface.

[sjp38]

FYI, we are trying to extend region-internal DAMOS fitlers for monitoring purpose[1]. To make it more useful, I think inclusive DAMOS filters are required. I will implement inclusive DAMOS filters and add support of it on damo.

[1] https://lore.kernel.org/[email protected]

[sjp38]

We are implementing DAMOS filter for passing specific memory through. It is still in progress, and not yet merged into mainline. It is available at DAMON development tree, though. We also added the support of it to damo, and pushed it to next branch: 2fc278c