Re: [RFC PATCH v2 0/7] DAMON based 2-tier memory management for CXL memory

[Honggyu Kim]

Hi SeongJae,

On Sun, 17 Mar 2024 12:13:57 -0700 SeongJae Park <sj@xxxxxxxxxx> wrote:
> On Sun, 17 Mar 2024 08:31:44 -0700 SeongJae Park <sj@xxxxxxxxxx> wrote:
> 
> > Hi Honggyu,
> > 
> > On Sun, 17 Mar 2024 17:36:29 +0900 Honggyu Kim <honggyu.kim@xxxxxx> wrote:
> > 
> > > 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 <sj@xxxxxxxxxx> wrote:
> > > > Hi Honggyu,
> [...]
> > > 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
> 
> [...]