Hi SeongJae,

On Sun, 17 Mar 2024 12:13:57 -0700 SeongJae Park [off-list ref] wrote:

On Sun, 17 Mar 2024 08:31:44 -0700 SeongJae Park [off-list ref] wrote:

quoted

Hi Honggyu,

On Sun, 17 Mar 2024 17:36:29 +0900 Honggyu Kim [off-list ref] wrote:

quoted

Hi SeongJae,

Thanks for the confirmation.  I have a few comments on young filter so
please read the inline comments again.

On Wed, 12 Mar 2024 08:53:00 -0800 SeongJae Park [off-list ref] wrote:

quoted

Hi Honggyu,

[...]

quoted

quoted

Thanks.  I see that it works fine, but I would like to have more
discussion about "young" filter.  What I think about filter is that if I
apply "young" filter "true" for demotion, then the action applies only
for "young" pages, but the current implementation works opposite.

I understand the function name of internal implementation is
"damos_pa_filter_out" so the basic action is filtering out, but the
cgroup filter works in the opposite way for now.

Does memcg filter works in the opposite way?  I don't think so because
__damos_pa_filter_out() sets 'matches' as 'true' only if the the given folio is
contained in the given memcg.  'young' filter also simply sets 'matches' as
'true' only if the given folio is young.

If it works in the opposite way, it's a bug that need to be fixed.  Please let
me know if I'm missing something.

I just read the DAMOS filters part of the documentation for DAMON sysfs
interface again.  I believe it is explaining the meaning of 'matching' as I
intended to, as below:

    You can write ``Y`` or ``N`` to ``matching`` file to filter out pages that does
    or does not match to the type, respectively.  Then, the scheme's action will
    not be applied to the pages that specified to be filtered out.

But, I found the following example for memcg filter is wrong, as below:

    For example, below restricts a DAMOS action to be applied to only non-anonymous
    pages of all memory cgroups except ``/having_care_already``.::
    
        # echo 2 > nr_filters
        # # filter out anonymous pages
        echo anon > 0/type
        echo Y > 0/matching
        # # further filter out all cgroups except one at '/having_care_already'
        echo memcg > 1/type
        echo /having_care_already > 1/memcg_path
        echo N > 1/matching

Specifically, the last line of the commands should write 'Y' instead of 'N' to
do what explained.  Without the fix, the action will be applied to only
non-anonymous pages of 'having_care_already' memcg.  This is definitely wrong.
I will fix this soon.  I'm unsure if this is what made you to believe memcg
DAMOS filter is working in the opposite way, though.

I got confused not because of this.  I just think it again that this
user interface is better to be more intuitive as I mentioned in the
previous thread.

Thanks,
Honggyu

Thanks,
SJ

[...]