From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out30-57.freemail.mail.aliyun.com (out30-57.freemail.mail.aliyun.com [115.124.30.57]) by sourceware.org (Postfix) with ESMTPS id AF3A83858D28 for ; Wed, 8 Dec 2021 02:14:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AF3A83858D28 X-Alimail-AntiSpam: AC=PASS; BC=-1|-1; BR=01201311R991e4; CH=green; DM=||false|; DS=||; FP=0|-1|-1|-1|0|-1|-1|-1; HT=e01e04357; MF=rongwei.wang@linux.alibaba.com; NM=1; PH=DS; RN=4; SR=0; TI=SMTPD_---0Uzq5BX._1638929671; Received: from 30.240.96.156(mailfrom:rongwei.wang@linux.alibaba.com fp:SMTPD_---0Uzq5BX._1638929671) by smtp.aliyun-inc.com(127.0.0.1); Wed, 08 Dec 2021 10:14:32 +0800 Message-ID: <53904314-f919-5965-a9cf-20c1b43eb47a@linux.alibaba.com> Date: Wed, 8 Dec 2021 10:14:30 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:95.0) Gecko/20100101 Thunderbird/95.0 Subject: Re: [PATCH RFC 1/1] elf: align the mapping address of LOAD segments with p_align Content-Language: en-US To: "H.J. Lu" Cc: GNU C Library , xuyu@linux.alibaba.com, gavin.dg@linux.alibaba.com References: <20211204045848.71105-1-rongwei.wang@linux.alibaba.com> <20211204045848.71105-2-rongwei.wang@linux.alibaba.com> From: Rongwei Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-21.0 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, UNPARSEABLE_RELAY, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2021 02:14:37 -0000 Hi hjl On 12/6/21 10:48 PM, H.J. Lu wrote: > On Fri, Dec 3, 2021 at 9:00 PM Rongwei Wang via Libc-alpha > wrote: >> >> Now, ld.so always map the LOAD segments and aligned by base >> page size (e.g. 4k in x86 or 4k, 16k and 64k in arm64). And >> this patch improve the scheme here. In this patch, ld.so >> can align the mapping address of the first LOAD segment with >> p_align when p_align is greater than the current base page >> size. > > This is a bug fix. Please open a glibc bug: > > https://sourceware.org/bugzilla/enter_bug.cgiOK And I requesting the account. > > with a testcase which should align variables to 2MB in the main By the way, I have a question about whether we need to align each LOAD segments? In our patch, we only fixed the mapping address for the first LOAD segment. > program and a shared library. Please include the testcase in > your patch and mention the glibc bug in your commit message. > >> And this change makes code segments using huge pages become >> simple and available. >> >> Signed-off-by: Rongwei Wang >> --- >> elf/dl-load.c | 1 + >> elf/dl-map-segments.h | 54 +++++++++++++++++++++++++++++++++++++++++-- >> include/link.h | 3 +++ >> 3 files changed, 56 insertions(+), 2 deletions(-) >> >> diff --git a/elf/dl-load.c b/elf/dl-load.c >> index e39980fb19..136cfe2fa8 100644 >> --- a/elf/dl-load.c >> +++ b/elf/dl-load.c >> @@ -1154,6 +1154,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, >> c->dataend = ph->p_vaddr + ph->p_filesz; >> c->allocend = ph->p_vaddr + ph->p_memsz; >> c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize)); >> + l->l_load_align = ph->p_align; >> >> /* Determine whether there is a gap between the last segment >> and this one. */ >> diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h >> index ac9f09ab4c..ae03236045 100644 >> --- a/elf/dl-map-segments.h >> +++ b/elf/dl-map-segments.h >> @@ -18,6 +18,47 @@ >> >> #include >> >> +static __always_inline void * >> +_dl_map_segments_align (const struct loadcmd *c, >> + ElfW(Addr) mappref, int fd, size_t alignment, >> + const size_t maplength) >> +{ >> + unsigned long map_start, map_start_align, map_end; >> + unsigned long maplen = (maplength >= alignment) ? >> + (maplength + alignment) : (2 * alignment); >> + >> + /* Allocate enough space to ensure that address aligned by >> + p_align is included. */ >> + map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplen, >> + PROT_NONE, >> + MAP_ANONYMOUS | MAP_PRIVATE, >> + -1, 0); >> + if (__glibc_unlikely ((void *) map_start == MAP_FAILED)) { >> + /* If mapping a aligned address failed, then ... */ >> + map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength, >> + c->prot, >> + MAP_COPY|MAP_FILE, >> + fd, c->mapoff); >> + >> + return (void *) map_start; >> + } >> + map_start_align = ALIGN_UP(map_start, alignment); >> + map_end = map_start_align + maplength; >> + >> + /* Remember which part of the address space this object uses. */ >> + map_start_align = (ElfW(Addr)) __mmap ((void *) map_start_align, maplength, >> + c->prot, >> + MAP_COPY|MAP_FILE|MAP_FIXED, >> + fd, c->mapoff); >> + if (__glibc_unlikely ((void *) map_start_align == MAP_FAILED)) >> + return MAP_FAILED; >> + if (map_start_align > map_start) >> + __munmap((void *)map_start, map_start_align - map_start); >> + __munmap((void *)map_end, map_start + maplen - map_end); >> + >> + return (void *) map_start_align; >> +} >> + > > Please follow the glibc coding format. > >> /* This implementation assumes (as does the corresponding implementation >> of _dl_unmap_segments, in dl-unmap-segments.h) that shared objects >> are always laid out with all segments contiguous (or with gaps >> @@ -52,11 +93,20 @@ _dl_map_segments (struct link_map *l, int fd, >> c->mapstart & GLRO(dl_use_load_bias)) >> - MAP_BASE_ADDR (l)); >> >> - /* Remember which part of the address space this object uses. */ >> - l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength, >> + /* During mapping, align the mapping address of the LOAD segments >> + according to own p_align. This helps OS map its code segment to >> + huge pages. */ >> + if (l->l_load_align > GLRO(dl_pagesize)) { >> + l->l_map_start = (ElfW(Addr)) _dl_map_segments_align (c, >> + mappref, fd, >> + l->l_load_align, maplength); >> + } else { >> + /* Remember which part of the address space this object uses. */ >> + l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength, >> c->prot, >> MAP_COPY|MAP_FILE, >> fd, c->mapoff); > > Please follow the glibc coding format. OK Thanks. > >> + } >> if (__glibc_unlikely ((void *) l->l_map_start == MAP_FAILED)) >> return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT; >> >> diff --git a/include/link.h b/include/link.h >> index aea268439c..fc6ce29fab 100644 >> --- a/include/link.h >> +++ b/include/link.h >> @@ -298,6 +298,9 @@ struct link_map >> >> /* Thread-local storage related info. */ >> >> + /* Alignment requirement of the LOAD block. */ >> + size_t l_load_align; >> + >> /* Start of the initialization image. */ >> void *l_tls_initimage; >> /* Size of the initialization image. */ >> -- >> 2.27.0 >> > > Thanks. >