linux-kernel - Re: [Question] Should we reuse target when damon's operation changed?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20221130232457.56003-1-sj@kernel.org>
Date:   Wed, 30 Nov 2022 23:24:57 +0000
From:   SeongJae Park <sj@...nel.org>
To:     Yun Levi <ppbuk5246@...il.com>
Cc:     SeongJae Park <sj@...nel.org>, damon@...ts.linux.dev,
        linux-mm@...ck.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [Question] Should we reuse target when damon's operation changed?

Hi Levi,

On Wed, 30 Nov 2022 15:01:29 +0900 Yun Levi <ppbuk5246@...il.com> wrote:

> Hello SJ.
> 
> While I try to use damon, I have some questions whether it is correct
> to reuse damon_target structure when damon's operation is changed.
> 
> At first, one user set up damon like below.
>     echo 1 > /sys/kernel/mm/damon/admin/kdamonds/nr_kdamonds
>     echo 1 > /sys/kernel/mm/damon/admin/kdamonds/0/contexts/nr_contexts
>     echo 1 > /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/targets/nr_targets
>     echo 1 > /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/targets/pid_target
>     echo 1 > /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/targets/regions/nr_regions
>     echo 0 > /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/targets/regions/nr_regions/0/start
>     echo 16384 >
> /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/targets/regions/nr_regions/0/end
> 
>     echo vaddr >  /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/operation
>     echo on >  /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/state
> 
> And some time pass, user change the operation as "paddr" like below:
>     echo paddr >  /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/operation
>     echo commit >  /sys/kernel/mm/damon/admin/kdamonds/0/contexts/0/state
> 
> In that situation, the damon_target is reused and the region isn't changed,
> former region information which damon_region has -- nr_accesse,
> last_nr_accesss, last_sample addr is kept.
> 
> But, former accessed information is based on vaddr and changed
> accessed information should be based on paddr and it seems the wrong
> information to new applied operation.
> 
> IIUC, it makes some confusion to kdamond when it merges or splits
> regions based on above information.
> 
> So, Is it much better to remove the target and region information when
> the operation is changed? or should we check whether it's possible to
> reuse former access information between former and new operation?

Thank you for the good question and the suggestion.  I agree that it could be
confusing.  Such a scenario could be just a mistake in many cases.  However,
I'm unsure how we could distinguish the mistake from intended usage.  That is,
the intended initial monitoring range for previous 'operation' and later one
could be really same in some cases, as DAMON doesn't ask it to be strict.

After all, DAMON allows users to set the values as much as they want before
committing the input, so this looks mistake of the user, not DAMON.  Of course
DAMON could take care of more things, but in my humble opinion, it's hard to
distinguish the mistake from the intended change in a simple way.  And I'm not
sure if this is a real critical issue that deserves to add some extent of
complexity.  So unless you know critical issue due to this or have a good way
for improving the situation in a simple way, please suggest.  If not, I'd
suggest to keep this in current simple form.

Nevertheless, maybe we could improve this situation by warning this case on the
documentation.  If you'd like to, please feel free to send a patch for that.

Thanks,
SJ

> 
> Thanks.
> 
> -- 
> Best regards,
> Levi