linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Konovalov <andreyknvl@gmail.com>
To: Marco Elver <elver@google.com>, Alexander Potapenko <glider@google.com>
Cc: andrey.konovalov@linux.dev, Dmitry Vyukov <dvyukov@google.com>,
	 Vlastimil Babka <vbabka@suse.cz>,
	kasan-dev@googlegroups.com,
	 Evgenii Stepanov <eugenis@google.com>,
	Oscar Salvador <osalvador@suse.de>,
	 Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	Andrey Konovalov <andreyknvl@google.com>
Subject: Re: [PATCH v3 00/19] stackdepot: allow evicting stack traces
Date: Fri, 3 Nov 2023 22:37:56 +0100	[thread overview]
Message-ID: <CA+fCnZeQ6nkCbkOR4GqGQ9OzprGNNrXvrOqqsJP0Vr3uJKLdrQ@mail.gmail.com> (raw)
In-Reply-To: <CANpmjNNoJQoWzODAbc4naq--b+LOfK76TCbx9MpL8+4x9=LTiw@mail.gmail.com>

On Tue, Oct 24, 2023 at 3:14 PM Marco Elver <elver@google.com> wrote:
>
> 1. I know fixed-sized slots are need for eviction to work, but have
> you evaluated if this causes some excessive memory waste now? Or is it
> negligible?

With the current default stack depot slot size of 64 frames, a single
stack trace takes up ~3-4x on average compared to precisely sized
slots (KMSAN is closer to ~4x due to its 3-frame-sized linking
records).

However, as the tag-based KASAN modes evict old stack traces, the
average total amount of memory used for stack traces is ~0.5 MB (with
the default stack ring size of 32k entries).

I also have just mailed an eviction implementation for Generic KASAN.
With it, the stack traces take up ~1 MB per 1 GB of RAM while running
syzkaller (stack traces are evicted when they are flushed from
quarantine, and quarantine's size depends on the amount of RAM.)

The only problem is KMSAN. Based on a discussion with Alexander, it
might not be possible to implement the eviction for it. So I suspect,
with this change, syzbot might run into the capacity WARNING from time
to time.

The simplest solution would be to bump the maximum size of stack depot
storage to x4 if KMSAN is enabled (to 512 MB from the current 128 MB).
KMSAN requires a significant amount of RAM for shadow anyway.

Would that be acceptable?

> If it turns out to be a problem, one way out would be to partition the
> freelist into stack size classes; e.g. one for each of stack traces of
> size 8, 16, 32, 64.

This shouldn't be hard to implement.

However, as one of the perf improvements, I'm thinking of saving a
stack trace directly into a stack depot slot (to avoid copying it).
With this, we won't know the stack trace size before it is saved. So
this won't work together with the size classes.

> 2. I still think switching to the percpu_rwsem right away is the right
> thing, and not actually a downside. I mentioned this before, but you
> promised a follow-up patch, so I trust that this will happen. ;-)

First thing on my TODO list wrt perf improvements :)

> Acked-by: Marco Elver <elver@google.com>
>
> The series looks good in its current state. However, see my 2
> higher-level comments above.

Thank you!


  reply	other threads:[~2023-11-03 21:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-23 16:22 andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 01/19] lib/stackdepot: check disabled flag when fetching andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 02/19] lib/stackdepot: simplify __stack_depot_save andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 03/19] lib/stackdepot: drop valid bit from handles andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 04/19] lib/stackdepot: add depot_fetch_stack helper andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 05/19] lib/stackdepot: use fixed-sized slots for stack records andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 06/19] lib/stackdepot: fix and clean-up atomic annotations andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 07/19] lib/stackdepot: rework helpers for depot_alloc_stack andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 08/19] lib/stackdepot: rename next_pool_required to new_pool_required andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 09/19] lib/stackdepot: store next pool pointer in new_pool andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 10/19] lib/stackdepot: store free stack records in a freelist andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 11/19] lib/stackdepot: use read/write lock andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 12/19] lib/stackdepot: use list_head for stack record links andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 13/19] kmsan: use stack_depot_save instead of __stack_depot_save andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 14/19] lib/stackdepot, kasan: add flags to __stack_depot_save and rename andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 15/19] lib/stackdepot: add refcount for records andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 16/19] lib/stackdepot: allow users to evict stack traces andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 17/19] kasan: remove atomic accesses to stack ring entries andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 18/19] kasan: check object_size in kasan_complete_mode_report_info andrey.konovalov
2023-10-23 16:22 ` [PATCH v3 19/19] kasan: use stack_depot_put for tag-based modes andrey.konovalov
2023-10-24  9:32 ` [PATCH v3 00/19] stackdepot: allow evicting stack traces Anders Roxell
2023-10-24 13:13 ` Marco Elver
2023-11-03 21:37   ` Andrey Konovalov [this message]
2023-11-06 17:41     ` Andrey Konovalov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+fCnZeQ6nkCbkOR4GqGQ9OzprGNNrXvrOqqsJP0Vr3uJKLdrQ@mail.gmail.com \
    --to=andreyknvl@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrey.konovalov@linux.dev \
    --cc=andreyknvl@google.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=osalvador@suse.de \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox