| Message ID | 20210917110756.1121272-1-elver@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] kfence: count unexpectedly skipped allocations | expand |
On Fri, 17 Sept 2021 at 13:08, 'Marco Elver' via kasan-dev <kasan-dev@googlegroups.com> wrote: > > Maintain a counter to count allocations that are skipped due to being > incompatible (oversized, incompatible gfp flags) or no capacity. > > This is to compute the fraction of allocations that could not be > serviced by KFENCE, which we expect to be rare. > > Signed-off-by: Marco Elver <elver@google.com> > --- > mm/kfence/core.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/mm/kfence/core.c b/mm/kfence/core.c > index 7a97db8bc8e7..2755800f3e2a 100644 > --- a/mm/kfence/core.c > +++ b/mm/kfence/core.c > @@ -112,6 +112,8 @@ enum kfence_counter_id { > KFENCE_COUNTER_FREES, > KFENCE_COUNTER_ZOMBIES, > KFENCE_COUNTER_BUGS, > + KFENCE_COUNTER_SKIP_INCOMPAT, > + KFENCE_COUNTER_SKIP_CAPACITY, > KFENCE_COUNTER_COUNT, > }; > static atomic_long_t counters[KFENCE_COUNTER_COUNT]; > @@ -121,6 +123,8 @@ static const char *const counter_names[] = { > [KFENCE_COUNTER_FREES] = "total frees", > [KFENCE_COUNTER_ZOMBIES] = "zombie allocations", > [KFENCE_COUNTER_BUGS] = "total bugs", > + [KFENCE_COUNTER_SKIP_INCOMPAT] = "skipped allocations (incompatible)", > + [KFENCE_COUNTER_SKIP_CAPACITY] = "skipped allocations (capacity)", > }; > static_assert(ARRAY_SIZE(counter_names) == KFENCE_COUNTER_COUNT); > > @@ -272,7 +276,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g > } > raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags); > if (!meta) > - return NULL; > + goto no_capacity; > > if (unlikely(!raw_spin_trylock_irqsave(&meta->lock, flags))) { > /* > @@ -289,7 +293,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g > list_add_tail(&meta->list, &kfence_freelist); > raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags); > > - return NULL; > + goto no_capacity; Do we expect this case to be so rare that we don't care? Strictly speaking it's not no_capacity. So if I see large no_capacity numbers, the first question I will have is: is it really no_capacity, or some other case that we mixed together? > } > > meta->addr = metadata_to_pageaddr(meta); > @@ -349,6 +353,10 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g > atomic_long_inc(&counters[KFENCE_COUNTER_ALLOCS]); > > return addr; > + > +no_capacity: > + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_CAPACITY]); > + return NULL; > } > > static void kfence_guarded_free(void *addr, struct kfence_metadata *meta, bool zombie) > @@ -740,8 +748,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) > * Perform size check before switching kfence_allocation_gate, so that > * we don't disable KFENCE without making an allocation. > */ > - if (size > PAGE_SIZE) > + if (size > PAGE_SIZE) { > + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]); > return NULL; > + } > > /* > * Skip allocations from non-default zones, including DMA. We cannot > @@ -749,8 +759,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) > * properties (e.g. reside in DMAable memory). > */ > if ((flags & GFP_ZONEMASK) || > - (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) > + (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) { > + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]); > return NULL; > + } > > /* > * allocation_gate only needs to become non-zero, so it doesn't make > -- > 2.33.0.464.g1972c5931b-goog > > -- > You received this message because you are subscribed to the Google Groups "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210917110756.1121272-1-elver%40google.com.
On Fri, 17 Sept 2021 at 14:58, Dmitry Vyukov <dvyukov@google.com> wrote: > > On Fri, 17 Sept 2021 at 13:08, 'Marco Elver' via kasan-dev > <kasan-dev@googlegroups.com> wrote: > > > > Maintain a counter to count allocations that are skipped due to being > > incompatible (oversized, incompatible gfp flags) or no capacity. > > > > This is to compute the fraction of allocations that could not be > > serviced by KFENCE, which we expect to be rare. > > > > Signed-off-by: Marco Elver <elver@google.com> > > --- > > mm/kfence/core.c | 20 ++++++++++++++++---- > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/mm/kfence/core.c b/mm/kfence/core.c > > index 7a97db8bc8e7..2755800f3e2a 100644 > > --- a/mm/kfence/core.c > > +++ b/mm/kfence/core.c > > @@ -112,6 +112,8 @@ enum kfence_counter_id { > > KFENCE_COUNTER_FREES, > > KFENCE_COUNTER_ZOMBIES, > > KFENCE_COUNTER_BUGS, > > + KFENCE_COUNTER_SKIP_INCOMPAT, > > + KFENCE_COUNTER_SKIP_CAPACITY, > > KFENCE_COUNTER_COUNT, > > }; > > static atomic_long_t counters[KFENCE_COUNTER_COUNT]; > > @@ -121,6 +123,8 @@ static const char *const counter_names[] = { > > [KFENCE_COUNTER_FREES] = "total frees", > > [KFENCE_COUNTER_ZOMBIES] = "zombie allocations", > > [KFENCE_COUNTER_BUGS] = "total bugs", > > + [KFENCE_COUNTER_SKIP_INCOMPAT] = "skipped allocations (incompatible)", > > + [KFENCE_COUNTER_SKIP_CAPACITY] = "skipped allocations (capacity)", > > }; > > static_assert(ARRAY_SIZE(counter_names) == KFENCE_COUNTER_COUNT); > > > > @@ -272,7 +276,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g > > } > > raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags); > > if (!meta) > > - return NULL; > > + goto no_capacity; > > > > if (unlikely(!raw_spin_trylock_irqsave(&meta->lock, flags))) { > > /* > > @@ -289,7 +293,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g > > list_add_tail(&meta->list, &kfence_freelist); > > raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags); > > > > - return NULL; > > + goto no_capacity; > > Do we expect this case to be so rare that we don't care? > Strictly speaking it's not no_capacity. So if I see large no_capacity > numbers, the first question I will have is: is it really no_capacity, > or some other case that we mixed together? Hmm, true. I think we can just ignore counting this -- I'd expect some bug-storm for this to become likely, at which point the system is in a pretty bad state anyway (and we see bug counts increasing). I'll remove this one. > > > > } > > > > meta->addr = metadata_to_pageaddr(meta); > > @@ -349,6 +353,10 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g > > atomic_long_inc(&counters[KFENCE_COUNTER_ALLOCS]); > > > > return addr; > > + > > +no_capacity: > > + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_CAPACITY]); > > + return NULL; > > } > > > > static void kfence_guarded_free(void *addr, struct kfence_metadata *meta, bool zombie) > > @@ -740,8 +748,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) > > * Perform size check before switching kfence_allocation_gate, so that > > * we don't disable KFENCE without making an allocation. > > */ > > - if (size > PAGE_SIZE) > > + if (size > PAGE_SIZE) { > > + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]); > > return NULL; > > + } > > > > /* > > * Skip allocations from non-default zones, including DMA. We cannot > > @@ -749,8 +759,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) > > * properties (e.g. reside in DMAable memory). > > */ > > if ((flags & GFP_ZONEMASK) || > > - (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) > > + (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) { > > + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]); > > return NULL; > > + } > > > > /* > > * allocation_gate only needs to become non-zero, so it doesn't make > > -- > > 2.33.0.464.g1972c5931b-goog > > > > -- > > You received this message because you are subscribed to the Google Groups "kasan-dev" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com. > > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210917110756.1121272-1-elver%40google.com.
diff --git a/mm/kfence/core.c b/mm/kfence/core.c index 7a97db8bc8e7..2755800f3e2a 100644 --- a/mm/kfence/core.c +++ b/mm/kfence/core.c @@ -112,6 +112,8 @@ enum kfence_counter_id { KFENCE_COUNTER_FREES, KFENCE_COUNTER_ZOMBIES, KFENCE_COUNTER_BUGS, + KFENCE_COUNTER_SKIP_INCOMPAT, + KFENCE_COUNTER_SKIP_CAPACITY, KFENCE_COUNTER_COUNT, }; static atomic_long_t counters[KFENCE_COUNTER_COUNT]; @@ -121,6 +123,8 @@ static const char *const counter_names[] = { [KFENCE_COUNTER_FREES] = "total frees", [KFENCE_COUNTER_ZOMBIES] = "zombie allocations", [KFENCE_COUNTER_BUGS] = "total bugs", + [KFENCE_COUNTER_SKIP_INCOMPAT] = "skipped allocations (incompatible)", + [KFENCE_COUNTER_SKIP_CAPACITY] = "skipped allocations (capacity)", }; static_assert(ARRAY_SIZE(counter_names) == KFENCE_COUNTER_COUNT); @@ -272,7 +276,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g } raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags); if (!meta) - return NULL; + goto no_capacity; if (unlikely(!raw_spin_trylock_irqsave(&meta->lock, flags))) { /* @@ -289,7 +293,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g list_add_tail(&meta->list, &kfence_freelist); raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags); - return NULL; + goto no_capacity; } meta->addr = metadata_to_pageaddr(meta); @@ -349,6 +353,10 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g atomic_long_inc(&counters[KFENCE_COUNTER_ALLOCS]); return addr; + +no_capacity: + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_CAPACITY]); + return NULL; } static void kfence_guarded_free(void *addr, struct kfence_metadata *meta, bool zombie) @@ -740,8 +748,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) * Perform size check before switching kfence_allocation_gate, so that * we don't disable KFENCE without making an allocation. */ - if (size > PAGE_SIZE) + if (size > PAGE_SIZE) { + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]); return NULL; + } /* * Skip allocations from non-default zones, including DMA. We cannot @@ -749,8 +759,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) * properties (e.g. reside in DMAable memory). */ if ((flags & GFP_ZONEMASK) || - (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) + (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) { + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]); return NULL; + } /* * allocation_gate only needs to become non-zero, so it doesn't make
Maintain a counter to count allocations that are skipped due to being incompatible (oversized, incompatible gfp flags) or no capacity. This is to compute the fraction of allocations that could not be serviced by KFENCE, which we expect to be rare. Signed-off-by: Marco Elver <elver@google.com> --- mm/kfence/core.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)