From patchwork Thu Sep 10 21:11:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 8625 Received: (qmail 97511 invoked by alias); 10 Sep 2015 21:11:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 97502 invoked by uid 89); 10 Sep 2015 21:11:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Subject: [PATCH v3] Don't fall back to mmap if there are still locked non-avoided arenas to try. To: Siddhesh Poyarekar , libc-alpha@sourceware.org, Josef Bacik , Mike Frysinger References: <1439974941-6577-1-git-send-email-siddhesh@redhat.com> <20150819135029.GC1584@vapier> <20150819172549.GN2415@spoyarek.pnq.redhat.com> From: "Carlos O'Donell" X-Enigmail-Draft-Status: N1110 Message-ID: <55F1F212.1040101@redhat.com> Date: Thu, 10 Sep 2015 17:11:46 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20150819172549.GN2415@spoyarek.pnq.redhat.com> On 08/19/2015 01:25 PM, Siddhesh Poyarekar wrote: > On Wed, Aug 19, 2015 at 09:50:29AM -0400, Mike Frysinger wrote: >> verified how ? > > Sorry, I meant to write "verified that there were no regressions on > x86_64". Testing this behaviour itself is tricky since you'll need to > generate the right amount of load to cause contention, with a single > arena or in a retry path. > >> doesn't this comment explicitly say you don't want to use the avoid arena ? >> doesn't it need updating now with this change in logic ? > > Right, updated patch with comment: The logic in reused_arena is still confused. For example we don't need a begin, since next_to_use is the start of our loop. Rather than correct this, I'm going to give you an alternate suggestion. From first principles we need only two loops. Loop 1: Hot loop. Find the first non-corrupt, non-locked, non-avoided arena. -> Return it (since we just locked it with trylock). Failing that. Loop 2: Slow loop. Find the first non-corrupt, non-avoided arena. -> Lock it (potentially waiting). -> Return it. Note: We do not inline slow loop into hot loop, even though we could and remember the last non-corrupt non-avoided arena, but that complicates the logic significantly. Failing that, all arenas are corrupt or avoided, so we return NULL to signal that other allocation schemes need to be used e.g. malloc. Note: avoid_arena are not locked as the comments say, it's always unlocked by arena_get_retry before looking to reuse an arena. What do you think about this? v3 - Split into hot/slow loops. - Rewrite logic. --- Cheers, Carlos. diff --git a/malloc/arena.c b/malloc/arena.c index b44e307..da94816 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -797,7 +804,7 @@ get_free_list (void) /* Lock and return an arena that can be reused for memory allocation. Avoid AVOID_ARENA as we have already failed to allocate memory in - it and it is currently locked. */ + that arena. */ static mstate reused_arena (mstate avoid_arena) { @@ -809,32 +816,34 @@ reused_arena (mstate avoid_arena) result = next_to_use; do { - if (!arena_is_corrupt (result) && !mutex_trylock (&result->mutex)) + if (!arena_is_corrupt (result) && + !mutex_trylock (&result->mutex) && + result != avoid_arena) goto out; result = result->next; } while (result != next_to_use); - /* Avoid AVOID_ARENA as we have already failed to allocate memory - in that arena and it is currently locked. */ - if (result == avoid_arena) - result = result->next; - - /* Make sure that the arena we get is not corrupted. */ - mstate begin = result; - while (arena_is_corrupt (result) || result == avoid_arena) + /* All arenas are locked, corrupted, or avoided. We start again looking for + any arena to use. This time we consider locked non-avoided arenas and + will wait for them instead of falling back to other allocation schemes. + We will not retry the avoided arena which just failed. */ + do { + if (!arena_is_corrupt (result) && + result != avoid_arena) + goto lock; + result = result->next; - if (result == begin) - break; } + while (result != next_to_use); - /* We could not find any arena that was either not corrupted or not the one - we wanted to avoid. */ - if (result == begin || result == avoid_arena) - return NULL; + /* All the arenas are corrupted or avoided. We have no choice but to return + NULL to signal that other fallbacks should be used. */ + return NULL; +lock: /* No arena available without contention. Wait for the next in line. */ LIBC_PROBE (memory_arena_reuse_wait, 3, &result->mutex, result, avoid_arena); (void) mutex_lock (&result->mutex);