From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 631261F466 for ; Tue, 14 Jan 2020 20:04:49 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=x/RhXObMwHM5gFa5 bReCKYTJ4q6Gohm9gXvFvPmjHKiOVMSOeaOCMy0ZjSwsNc0AJu1QX0ZKuQWJvSha O9Z5LAzqsjDuwYdk9HXt8IKRaFuPM4XncL9vapDunGj9wr6yeHm+v4y7GjrA/3SX F/Jt38mbjPU4li7WbMi7r2nKJ2Y= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=S9iccMFYUPi6ukjQ2nf1Qs AVGgg=; b=nOWn8KFOr8xUl2LEim6dtv5rEaCqJO2aH8pCD+belmUgeu/XKc0SOb 5afIVCyDxesOD8K/chv7kNMj2VBTnSXWzi6qIlYtpmWHbcLVzQWERlt+AoLK0Dj3 C6+EMnSZwjHDokskWF4z1tTaURHVaRKMWkdtJdkcK++CZFA83PCTg= Received: (qmail 22206 invoked by alias); 14 Jan 2020 20:04:46 -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 Received: (qmail 22196 invoked by uid 89); 14 Jan 2020 20:04:46 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-qt1-f196.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:references:from:autocrypt:subject:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=L7Th0Z9A6iErT3v8m5WluCWs9M1MJnsDdbN2+yL88yk=; b=q24P7D6TJaT9iO9ilWj6Ci2n4r2ikE5EeqWMar561LkQLIDEsdfKDNgt9dkBTD5iRf E8Yzrh6EUofosNA7MXtRokzVlVrncs3nmF3nwVF1ShhsQHzK9cGdFYA4nm7ZXD4AxrhU sWSaHq8911kGtE3LcfN35wexA+IjDjv9wcRBI4Ot2eogJ7RqJjGgnD76KCXmMU6IkBPu iQTKiBYWD7mPVjwKGshmLsfByVIVLVjxkqQCw5WlTqxQphvcBE4iR7IQ8/TEM3LinEv7 htfCotfLphO3UcwIePDMTkDGye2YomaKFSUcSx1WecrHQNhrrhczESTVh9H/nr/HfFNs 8Itg== To: libc-alpha@sourceware.org References: <1575394197-18006-1-git-send-email-david.kilroy@arm.com> <1575394197-18006-4-git-send-email-david.kilroy@arm.com> From: Adhemerval Zanella Subject: Re: [PATCH v3 3/3] elf: avoid stack allocation in dl_open_worker Message-ID: <76118723-523d-18c3-938f-2b12771395b5@linaro.org> Date: Tue, 14 Jan 2020 17:04:31 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <1575394197-18006-4-git-send-email-david.kilroy@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 03/12/2019 14:30, David Kilroy wrote: > As the sort was removed, there's no need to keep a separate map of > links. Instead, when relocating objects iterate over l_initfini > directly. > > This allows us to remove the loop copying l_initfini elements into > map. We still need a loop to identify the first and last elements that > need relocation. > > Tested by running the testsuite on x86_64. LGTM, thanks. Reviewed-by: Adhemerval Zanella > --- > elf/dl-open.c | 28 ++++++++++++---------------- > 1 file changed, 12 insertions(+), 16 deletions(-) > > diff --git a/elf/dl-open.c b/elf/dl-open.c > index c4d09c7c..eb36a91 100644 > --- a/elf/dl-open.c > +++ b/elf/dl-open.c > @@ -636,25 +636,18 @@ dl_open_worker (void *a) > /* Sort the objects by dependency for the relocation process. This > allows IFUNC relocations to work and it also means copy > relocation of dependencies are if necessary overwritten. */ I think we need to update the comment to state this is not sorting anymore, but rather using the already sorted list (done by _dl_map_object_deps). > - unsigned int nmaps = 0; > + unsigned int first = UINT_MAX; > + unsigned int last = 0; > unsigned int j = 0; > struct link_map *l = new->l_initfini[0]; > do > { > if (! l->l_real->l_relocated) > - ++nmaps; > - l = new->l_initfini[++j]; > - } > - while (l != NULL); > - /* Stack allocation is limited by the number of loaded objects. */ > - struct link_map *maps[nmaps]; > - nmaps = 0; > - j = 0; > - l = new->l_initfini[0]; > - do > - { > - if (! l->l_real->l_relocated) > - maps[nmaps++] = l; > + { > + if (first == UINT_MAX) > + first = j; > + last = j + 1; > + } > l = new->l_initfini[++j]; > } > while (l != NULL); Ok, it sets the range [first, last] as the potential maps that requires relocation. > @@ -669,9 +662,12 @@ dl_open_worker (void *a) > them. However, such relocation dependencies in IFUNC resolvers > are undefined anyway, so this is not a problem. */ > > - for (unsigned int i = nmaps; i-- > 0; ) > + for (unsigned int i = last; i-- > first; ) > { > - l = maps[i]; > + l = new->l_initfini[i]; > + > + if (l->l_real->l_relocated) > + continue; > > if (! relocation_in_progress) > { > Ok, some objects objects might already relocated.