On Fri, Nov 3, 2023 at 10:37 PM Andrey Konovalov <andreyknvl@xxxxxxxxx> wrote: > > On Tue, Oct 24, 2023 at 3:14 PM Marco Elver <elver@xxxxxxxxxx> 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. On a second thought, saving stack traces directly into a stack depot slot will require taking the write lock, which will badly affect performance, or using some other elaborate locking scheme, which might be an overkill. > > 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@xxxxxxxxxx> > > > > The series looks good in its current state. However, see my 2 > > higher-level comments above. > > Thank you!
