linux-kernel - Re: [PATCH v31 05/13] mm/damon: Implement primitives for the virtual memory address spaces

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Jun 2021 07:42:44 -0700
From:   Shakeel Butt <shakeelb@...gle.com>
To:     SeongJae Park <sj38.park@...il.com>
Cc:     SeongJae Park <sjpark@...zon.de>, Jonathan.Cameron@...wei.com,
        acme@...nel.org, alexander.shishkin@...ux.intel.com,
        amit@...nel.org, benh@...nel.crashing.org,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        David Hildenbrand <david@...hat.com>, dwmw@...zon.com,
        Marco Elver <elver@...gle.com>, "Du, Fan" <fan.du@...el.com>,
        foersleo@...zon.de, greg@...ah.com,
        Greg Thelen <gthelen@...gle.com>, guoju.fgj@...baba-inc.com,
        jgowans@...zon.com, Mel Gorman <mgorman@...e.de>, mheyne@...zon.de,
        Minchan Kim <minchan@...nel.org>,
        Ingo Molnar <mingo@...hat.com>, namhyung@...nel.org,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Rik van Riel <riel@...riel.com>,
        David Rientjes <rientjes@...gle.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mike Rapoport <rppt@...nel.org>, Shuah Khan <shuah@...nel.org>,
        sieberf@...zon.com, snu@...le79.org,
        Vlastimil Babka <vbabka@...e.cz>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        zgf574564920@...il.com, linux-damon@...zon.com,
        Linux MM <linux-mm@...ck.org>, linux-doc@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v31 05/13] mm/damon: Implement primitives for the virtual
 memory address spaces

On Thu, Jun 24, 2021 at 3:26 AM SeongJae Park <sj38.park@...il.com> wrote:
>
[...]
> > > +/*
> > > + * Get the three regions in the given target (task)
> > > + *
> > > + * Returns 0 on success, negative error code otherwise.
> > > + */
> > > +static int damon_va_three_regions(struct damon_target *t,
> > > +                               struct damon_addr_range regions[3])
> > > +{
> > > +       struct mm_struct *mm;
> > > +       int rc;
> > > +
> > > +       mm = damon_get_mm(t);
> > > +       if (!mm)
> > > +               return -EINVAL;
> > > +
> > > +       mmap_read_lock(mm);
> > > +       rc = __damon_va_three_regions(mm->mmap, regions);
> > > +       mmap_read_unlock(mm);
> >
> > This is being called for each target every second by default. Seems
> > too aggressive. Applications don't change their address space every
> > second. I would recommend to default ctx->primitive_update_interval to
> > a higher default value.
>
> Good point.  If there are many targets and each target has a huge number of
> VMAs, the overhead could be high.  Nevertheless, I couldn't find the overhead
> in my test setup.  Also, it seems someone are already started exploring DAMON
> patchset with the default value. and usages from others.  Silently changing the
> default value could distract such people.  So, if you think it's ok, I'd like
> to change the default value only after someone finds the overhead from their
> usages and asks a change.
>
> If you disagree or you found the overhead from your usage, please feel free to
> let me know.
>

mmap lock is a source contention in the real world workloads. We do
observe in our fleet and many others (like Facebook) do complain on
this issue. This is the whole motivation behind SFP, maple tree and
many other mmap lock scalability work. I would be really careful to
add another source of contention on mmap lock. Yes, the user can
change this interval themselves but we should not burden them with
this internal knowledge like "oh if you observe high mmap contention
you may want to increase this specific interval". We should set a good
default value to avoid such situations (most of the time).