From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out30-133.freemail.mail.aliyun.com (out30-133.freemail.mail.aliyun.com [115.124.30.133]) by sourceware.org (Postfix) with ESMTPS id 9A9293858C60 for ; Wed, 8 Dec 2021 03:04:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9A9293858C60 X-Alimail-AntiSpam: AC=PASS; BC=-1|-1; BR=01201311R151e4; CH=green; DM=||false|; DS=||; FP=0|-1|-1|-1|0|-1|-1|-1; HT=e01e04426; MF=rongwei.wang@linux.alibaba.com; NM=1; PH=DS; RN=4; SR=0; TI=SMTPD_---0UzqFB7-_1638932692; Received: from 30.240.96.156(mailfrom:rongwei.wang@linux.alibaba.com fp:SMTPD_---0UzqFB7-_1638932692) by smtp.aliyun-inc.com(127.0.0.1); Wed, 08 Dec 2021 11:04:53 +0800 Message-ID: Date: Wed, 8 Dec 2021 11:04:52 +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> <53904314-f919-5965-a9cf-20c1b43eb47a@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.2 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 03:04:58 -0000 On 12/8/21 10:33 AM, H.J. Lu wrote: > On Tue, Dec 7, 2021 at 6:14 PM Rongwei Wang > wrote: >> >> 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. > > I think the first one should be sufficient. You can verify it with a > 2MB aligned variable in PIE: > > [hjl@gnu-cfl-2 tmp]$ cat x.c > #include > > int foo __attribute__((aligned(0x200000))) = 1; > > int > main () > { > printf ("foo: %p\n", &foo); > } > [hjl@gnu-cfl-2 tmp]$ gcc -no-pie x.c > [hjl@gnu-cfl-2 tmp]$ ./a.out > foo: 0x800000 > [hjl@gnu-cfl-2 tmp]$ gcc x.c -fpie -pie > [hjl@gnu-cfl-2 tmp]$ ./a.out > foo: 0x55c529afe000 > [hjl@gnu-cfl-2 tmp]$ Learned it! Thanks. > >>> 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. >>> > > >