Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220901044249.4624-1-osalvador@suse.de> <20220901044249.4624-2-osalvador@suse.de>
 <YxBsWu36eqUw03Dy@elver.google.com> <YxBvcDFSsLqn3i87@dhcp22.suse.cz>
In-Reply-To: <YxBvcDFSsLqn3i87@dhcp22.suse.cz>
From:   Marco Elver <elver@google.com>
Date:   Thu, 1 Sep 2022 11:18:19 +0200
Message-ID: <CANpmjNNjkgibnBcp7ZOWGC5CcBJ=acgrRKo0cwZG0xOB5OCpLw@mail.gmail.com>
Subject: Re: [PATCH 1/3] lib/stackdepot: Add a refcount field in stack_record
To:     Michal Hocko <mhocko@suse.com>
Cc:     Oscar Salvador <osalvador@suse.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Vlastimil Babka <vbabka@suse.cz>,
        Eric Dumazet <edumazet@google.com>,
        Waiman Long <longman@redhat.com>,
        Suren Baghdasaryan <surenb@google.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>, kasan-dev@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 1 Sept 2022 at 10:38, Michal Hocko <mhocko@suse.com> wrote:
>
> On Thu 01-09-22 10:24:58, Marco Elver wrote:
> > On Thu, Sep 01, 2022 at 06:42AM +0200, Oscar Salvador wrote:
> [...]
> > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> > > index 5ca0d086ef4a..aeb59d3557e2 100644
> > > --- a/lib/stackdepot.c
> > > +++ b/lib/stackdepot.c
> > > @@ -63,6 +63,7 @@ struct stack_record {
> > >     u32 hash;                       /* Hash in the hastable */
> > >     u32 size;                       /* Number of frames in the stack */
> > >     union handle_parts handle;
> > > +   refcount_t count;               /* Number of the same repeated stacks */
> >
> > This will increase stack_record size for every user, even if they don't
> > care about the count.
>
> Couldn't this be used for garbage collection?

Only if we can precisely figure out at which point a stack is no
longer going to be needed.

But more realistically, stack depot was designed to be simple. Right
now it can allocate new stacks (from an internal pool), but giving the
memory back to that pool isn't supported. Doing garbage collection
would effectively be a redesign of stack depot. And for the purpose
for which stack depot was designed (debugging tools), memory has never
been an issue (note that stack depot also has a fixed upper bound on
memory usage).

We had talked (in the context of KASAN) about bounded stack storage,
but the preferred solution is usually a cache-based design which
allows evictions (in the simplest case a ring buffer), because
figuring out (and relying on) where precisely a stack will
definitively no longer be required in bug reports is complex and does
not guarantee the required bound on memory usage. Andrey has done the
work on this for tag-based KASAN modes:
https://lore.kernel.org/all/cover.1658189199.git.andreyknvl@google.com/