linux-kernel - Re: [PATCH 4/4] mm/damon/vaddr: indicate the target is invalid when 'nr

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 14 Sep 2022 16:21:35 +0800
From:   Kaixu Xia <xiakaixu1987@...il.com>
To:     SeongJae Park <sj@...nel.org>
Cc:     akpm@...ux-foundation.org, damon@...ts.linux.dev,
        linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>,
        Kaixu Xia <kaixuxia@...cent.com>
Subject: Re: [PATCH 4/4] mm/damon/vaddr: indicate the target is invalid when
 'nr_regions' is zero

On Wed, Sep 14, 2022 at 4:04 PM SeongJae Park <sj@...nel.org> wrote:
>
> On Wed, 14 Sep 2022 12:02:05 +0800 Kaixu Xia <xiakaixu1987@...il.com> wrote:
>
> > On Tue, Sep 13, 2022 at 11:11 PM SeongJae Park <sj@...nel.org> wrote:
> > >
> > > On Tue, 13 Sep 2022 17:11:27 +0800 xiakaixu1987@...il.com wrote:
> > >
> > > > From: Kaixu Xia <kaixuxia@...cent.com>
> > > >
> > > > When 'init()' and 'update()' DAMON operations failed and the number
> > > > of the damon_target regions is zero,
> > >
> > > Well, I think that could be a temporal failure.  In the case, later call of
> > > 'update()' could success?
> > >
> > Yeah, the kdamond while() loop calls 'update()' periodically to fix this
> > temporary failure. But for extreme scenarios that 'update()' continues to fail,
> > we should have some ways to detect this case.
>
> Even in the case, kdamond will do nothing but continuing the main loop while
> sleeping sample_aggr interval (5ms by default) for each iteration, and calling
> 'update()' for every update interval (100ms by default).  Waste is waste, but I
> don't think that's a real issue.  Further, continuous 'update()' failures mean
> the process is in some weird state anyway, so I'd assume the process would be
> finished soon.  kdamond will also finish as soon as the process finishes.
> Users could also find the strange situation (nothing in the monitoring results)
> and finish kdamond on their own.
>
> Anything I'm missing?
>
That sounds reasonable. Thanks for your detailed explanation.


Thanks,
Kaixu
>
> Andrew, I found you merged this patch in mm-unstable.  Could you please hold it
> until we finish this discussion?
>
>
> Thanks,
> SJ
>
> >
> > Thanks,
> > Kaixu
> > >
> > > Thanks,
> > > SJ
> > >
> > > > the kdamond would do nothing
> > > > to this monitoring target in this case. It makes no sense to run
> > > > kdamond when all of monitoring targets have no regions. So add the
> > > > judgement in 'target_valid()' operation to indicate the target is
> > > > invalid when 'nr_regions' is zero.
> > >
> > > >
> > > > Signed-off-by: Kaixu Xia <kaixuxia@...cent.com>
> > > > ---
> > > >  mm/damon/vaddr.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > > > index 39ea48d9cc15..65ff98d49ec0 100644
> > > > --- a/mm/damon/vaddr.c
> > > > +++ b/mm/damon/vaddr.c
> > > > @@ -598,6 +598,9 @@ static bool damon_va_target_valid(void *target)
> > > >       struct damon_target *t = target;
> > > >       struct task_struct *task;
> > > >
> > > > +     if (!damon_nr_regions(t))
> > > > +             return false;
> > > > +
> > > >       task = damon_get_task_struct(t);
> > > >       if (task) {
> > > >               put_task_struct(task);
> > > > --
> > > > 2.27.0
> >