From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Cyrill Gorcunov <gorcunov@gmail.com> Cc: tml <tarantool-patches@freelists.org>, Alexander Turenko <alexander.turenko@tarantool.org> Subject: Re: [PATCH v3] slab_arena: Enhance slab_arena_create to support madvise hints Date: Wed, 15 May 2019 16:08:12 +0300 [thread overview] Message-ID: <20190515130812.tvkrwnjclasvrbgn@esperanza> (raw) In-Reply-To: <20190515125304.GC2952@uranus.lan> On Wed, May 15, 2019 at 03:53:04PM +0300, Cyrill Gorcunov wrote: > On Wed, May 15, 2019 at 03:37:56PM +0300, Vladimir Davydov wrote: > > > > I'd also patch all places (tests, etc) where old flags (MAP_*) are used > > to switch them to new flags. Sooner or later we should drop this compat > > layer. > > Maybe we should do this in a separate patch? I would like the tests to sit > for some time with MAP_ flags to make sure everything works. And I'll prepare > the patch covering MAP_ next week, sounds OK? Okay. > > > > -#if !defined(MAP_ANONYMOUS) > > > -#define MAP_ANONYMOUS MAP_ANON > > > +static void > > > +madvise_checked(void *ptr, size_t size, int flags) > > > +{ > > > +#ifdef TARANTOOL_SMALL_USE_MADVISE > > > > It would be nice to have a warning in case MADV_DONTDUMP is unavailable. > > It should be printed only once. May be, here, or, even better, in > > Tarantool itself (via say_warn()). > > You know I thought if we could use small/conf.h inside tarantool. > For this sake I prefixed symbols with TARANROOL_SMALL_ to not interfere > with any other symbols. > > Another option -- provide something like > > static const uint64_t small_features = (bitset of features); I would define HAVE_MADV_DONTNEED in tarantool's config.h. > > bool small_have_feature(uint64_t mask) > { > return small_features & mask; > } > > ? > > > > > > > static void > > > munmap_checked(void *addr, size_t size) > > > { > > > if (munmap(addr, size)) { > > > char buf[64]; > > > - intptr_t ignore_it = (intptr_t)strerror_r(errno, buf, > > > - sizeof(buf)); > > > + char *ignore_it = strerror_r(errno, buf, sizeof(buf)); > > > > This wouldn't compile on OS X. Please leave it as is. Same's fair for > > madvise_checked. > > The problem is that if I leave it as is it doesn't compile on linux, > due to _GNU_SOURCE definition, which is required for MADV_ flags :/ It does compile on my box, which runs Linux. Note the cast: it should be fine to explicitly cast (char *) to intptr_t.
next prev parent reply other threads:[~2019-05-15 13:08 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-13 20:01 Cyrill Gorcunov 2019-05-15 12:37 ` Vladimir Davydov 2019-05-15 12:53 ` Cyrill Gorcunov 2019-05-15 13:08 ` Vladimir Davydov [this message] 2019-05-15 13:22 ` Cyrill Gorcunov
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=20190515130812.tvkrwnjclasvrbgn@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=alexander.turenko@tarantool.org \ --cc=gorcunov@gmail.com \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH v3] slab_arena: Enhance slab_arena_create to support madvise hints' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox