From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 6 May 2019 13:38:36 +0300 From: Vladimir Davydov Subject: Re: [PATCH 2/3] slab_arena: Provide slab_arena_madvise_create to madvice slabs Message-ID: <20190506103836.gxvrre4upkga7tej@esperanza> References: <20190501155006.14546-1-gorcunov@gmail.com> <20190501155006.14546-3-gorcunov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190501155006.14546-3-gorcunov@gmail.com> To: Cyrill Gorcunov Cc: tml , Alexander Turenko List-ID: [Cc += Alexander re tarantool/small backward compatibility] On Wed, May 01, 2019 at 06:50:05PM +0300, Cyrill Gorcunov wrote: > The purpose of this helper is to be able to eliminate > some of slab arenas from dumping, especially when huge > amount of memory involves. > > For this sake one should call slab_arena_madvise_create > right after area's creation. Strictly speaking this helper > may be called in any time but then some of already created > slabs which are not inside preallocated area won't be > madvised. > > Part of #3509 Again, please fix the link. > --- > https://github.com/tarantool/tarantool/issues/3509 > > small/slab_arena.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++ > small/slab_arena.h | 27 +++++++++++++++++++++++++ > 2 files changed, 76 insertions(+) > > diff --git a/small/slab_arena.c b/small/slab_arena.c > index 26e2620..12f6198 100644 > --- a/small/slab_arena.c > +++ b/small/slab_arena.c > @@ -149,6 +149,9 @@ slab_arena_create(struct slab_arena *arena, struct quota *quota, > arena->used = 0; > > arena->flags = flags; > +#ifdef TARANTOOL_SMALL_HAS_MADVISE > + pm_atomic_store(&arena->advice, MADV_NORMAL); > +#endif > > if (arena->prealloc) { > arena->arena = mmap_checked(arena->prealloc, > @@ -205,6 +208,12 @@ slab_map(struct slab_arena *arena) > __sync_sub_and_fetch(&arena->used, arena->slab_size); > quota_release(arena->quota, arena->slab_size); > } > + > +#ifdef TARANTOOL_SMALL_HAS_MADVISE > + if (ptr && pm_atomic_load(&arena->advice) != MADV_NORMAL) > + madvise(ptr, arena->slab_size, arena->advice); > +#endif > + Why use atomic? The arena cannot be used until it's fully initialized. > VALGRIND_MAKE_MEM_UNDEFINED(ptr, arena->slab_size); > return ptr; > } > @@ -226,3 +235,43 @@ slab_arena_mprotect(struct slab_arena *arena) > if (arena->arena) > mprotect(arena->arena, arena->prealloc, PROT_READ); > } > + > +#ifdef TARANTOOL_SMALL_HAS_MADVISE > +/** > + * slab_arena_madvise_create - Initialize madvise permanent strategy > + * @arena: slab arena to madvise > + * @advice: strategy (MADV_DONTDUMP, ...) > + * > + * This function setups @slab_arena strategy so that new memory > + * allocation with mmap() call gets madvise() call with @advice. > + * > + * Note that we don't allow to change strategy once choosen because > + * otherwise we need to walk over all possible slab cache entry. > + * > + * This function should be called right after the slab arena creation, > + * otherwise if called later some of the non-preallocated slabs won't be > + * madvise'd. > + */ > +int > +slab_arena_madvise_create(struct slab_arena *arena, int advice) I don't see much point in changing madvise settings on the fly. Let's instead allow to pass DONTDUMP flag at creation time. This means we have to combine MAP_ and MADV_ flags. I guess it was a bad idea to pass MAP_ flags directly to small_create() in the first place. What about introducing small-specific flags like SLAB_ARENA_SHARED SLAB_ARENA_DONTDUMP Come to think of it, I assume we don't even need to introduce SLAB_ARENA_SHARED - we always pass MAP_PRIVATE to small_create(). Let's drop it, perhaps? It will break backward compatibility, but I don't think it really matters, as tarantool/small is only used by tarantool - it isn't a big deal if we change its API. We only need to bump the version number, I guess. Alexander, please amend me if I'm wrong. > +{ > + int expected = MADV_NORMAL; > + > + if (!pm_atomic_compare_exchange_strong(&arena->advice, > + &expected, advice)) { > + return -EBUSY; > + } > + > + if (arena->arena) > + return madvise(arena->arena, arena->prealloc, advice); > + return 0; > +} > +#else > +int > +slab_arena_madvise_create(struct slab_arena *arena, int advice) > +{ > + (void)arena; > + (void)advice; > + return -EINVAL; > +} > +#endif /* TARANTOOL_SMALL_HAS_MADVISE */ > diff --git a/small/slab_arena.h b/small/slab_arena.h > index 364252f..4d86545 100644 > --- a/small/slab_arena.h > +++ b/small/slab_arena.h > @@ -31,8 +31,10 @@ > * SUCH DAMAGE. > */ > #include "lf_lifo.h" > +#include "config.h" > #include > #include > +#include > > #if defined(__cplusplus) > extern "C" { > @@ -97,6 +99,12 @@ struct slab_arena { > * mmap() flags: MAP_SHARED or MAP_PRIVATE > */ > int flags; > +#ifdef TARANTOOL_SMALL_HAS_MADVISE > + /** > + * madvise() flag > + */ > + int advice; > +#endif > }; > > /** Initialize an arena. */ > @@ -120,6 +128,25 @@ slab_unmap(struct slab_arena *arena, void *ptr); > void > slab_arena_mprotect(struct slab_arena *arena); > > +/** madvise() all slabs. */ > +int > +slab_arena_madvise_create(struct slab_arena *arena, int advice); > + > +/** handy helpers for slab_arena_madvise_create. */ > +#ifdef TARANTOOL_SMALL_HAS_MADVISE > +static inline int > +slab_arena_madvise_dontdump(struct slab_arena *arena) > +{ > + return slab_arena_madvise_create(arena, MADV_DONTDUMP); > +} > +#else > +static inline int > +slab_arena_madvise_dontdump(struct slab_arena *arena) > +{ > + return -EINVAL; > +} > +#endif > + > /** > * Align a size - round up to nearest divisible by the given alignment. > * Alignment must be a power of 2