Message ID | 20221123161805.1579-1-jszhang@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp2895159wrr; Wed, 23 Nov 2022 08:37:40 -0800 (PST) X-Google-Smtp-Source: AA0mqf4Qd6ToGn27fqxbMPbKydfF4eyjRVAtHVT2UGL3A/EzU946pIngrH2mWZQGldNxrmJIS7+I X-Received: by 2002:a05:6402:cf:b0:458:a1bb:4c9a with SMTP id i15-20020a05640200cf00b00458a1bb4c9amr26591682edu.121.1669221460529; Wed, 23 Nov 2022 08:37:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669221460; cv=none; d=google.com; s=arc-20160816; b=vKssBTqt/Moi4Zlx/5e8q64KG2Gu6kbvcZRHWcocfGmF69kXgtVI+Ke/YG2F0eHyuI JisU9Euq3DMgfpQw0cqsRR8tQiCpjI1KETqntb24siTJexNxw5DjffLEeg2xkLzSh8Ly 8jIAPVT1R1l10r130eQfuxjisXPFgCPPN3CU5+8WpUov4+6rO4KzoaY8xRVO+hbjHAAk o0u0T5h3lIZRbienNLpq4b41jP5qYfny7WWbM8R5eSiK1mpecStTogbSaHcJ9SVHGtxM a2s335n7JkuHwlaw9meFkvkhkHwm8UYBVtXBOfuy7O+NvJUmWh43qmafB4R25z8z9e7r /byg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=8S9bPJVwnByMjueVMPRQY+tH8+5wgYOYQ6hUt3Z1VFM=; b=hU6JL3uyKxEMzKtyM2x8L9CsA42b4h7gXd5R+UCtgF+Vhhmx0fVbTMxpXyUADxrgST Uhm8IzMPYQQ9E8QnTPV1sWIqXbTePsdGq86qGKRhf9FJmxjNGzTl42ZEVLbL0+RUh4OS FIwv4LVZ+RAdpn7zyMe138tO4k9XRQwcpwC9lTVCxyjLivIGj05QxFUVwmbgvkDFx8R9 ypgw3zBaD7GjPKx5eEMWJYuW2ZERmHUALlASTdyg5u6Yj5HHe+K/oLITpiXLK36mQrXR hWCYXjh6JskvOCmhiUrUDjXZEuviJvr/QgtDuTMODrHw+qWIPBbL9y77AL8rRC5tDKP5 WN/Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=gTvnwSCY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k10-20020aa7d2ca000000b00469acead38esi6428938edr.1.2022.11.23.08.37.14; Wed, 23 Nov 2022 08:37:40 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=gTvnwSCY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239162AbiKWQ2j (ORCPT <rfc822;fengqi706@gmail.com> + 99 others); Wed, 23 Nov 2022 11:28:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53330 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239135AbiKWQ2A (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 23 Nov 2022 11:28:00 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8F49129CBF for <linux-kernel@vger.kernel.org>; Wed, 23 Nov 2022 08:27:56 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 2B67761DE6 for <linux-kernel@vger.kernel.org>; Wed, 23 Nov 2022 16:27:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 159E0C433C1; Wed, 23 Nov 2022 16:27:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1669220875; bh=RbFg5SUvS3xFJJRTRaVHi2bX1Rc8bI0Z/xmm4da0qQw=; h=From:To:Cc:Subject:Date:From; b=gTvnwSCYZ0wF4boR8aVWXndodyYNlX7xtCHMDLuAKCVA9rq6Kcy+XPbwZB+efS2sP ePy9soNF95TLT+xdeDx5hY66ofX4M9nWWEtUP2oVJkcIDl6CXtAINIpYPyHA5sRVjq +PLEp5/RNeu7JXuUlmd81XnM3hbOYziJKTKrJcI/BT/esU864J02NRp4hvzm/HtR68 DayvsbEUfIBGyYFs9YEY1pUqFA3L9wWbqLw4FzJ7OkhXzCSSeUgnZomdKkrBtthhJp WmzttKvBIcFq5ymMgoGJk+JCunnO3LDK/QIHXlX1NveT3hhFXckFAc2NgBNRus6z2a W8kK6Qt0M+IDg== From: Jisheng Zhang <jszhang@kernel.org> To: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu> Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, Andrew Jones <ajones@ventanamicro.com> Subject: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr Date: Thu, 24 Nov 2022 00:18:05 +0800 Message-Id: <20221123161805.1579-1-jszhang@kernel.org> X-Mailer: git-send-email 2.37.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750305562290011624?= X-GMAIL-MSGID: =?utf-8?q?1750305562290011624?= |
Series |
[RFC] riscv: vdso: remove hardcoded 0x800 .text section start addr
|
|
Commit Message
Jisheng Zhang
Nov. 23, 2022, 4:18 p.m. UTC
It seems the hardcoded 0x800 isn't necessary, but removing it brings a
small vdso.so and aligns with other architectures.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
arch/riscv/kernel/vdso/vdso.lds.S | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
Comments
On Thu, Nov 24, 2022 at 12:18:05AM +0800, Jisheng Zhang wrote: > It seems the hardcoded 0x800 isn't necessary, but removing it brings a s/, but/and/ > small vdso.so and aligns with other architectures. This commit message didn't really satisfy my desire to understand why the comment and '. = 0x800' were there in the first place and if its safe to remove now, so I tried to do some of my own digging. I found commit 5b9304933730 ("x86 vDSO: generate vdso-syms.lds") commit f6b46ebf904f ("x86 vDSO: new layout") which removes the comment and hard coding for x86 by changing the vdso Makefile. Then looking at commit 9031fefde6f2 ("arm64: VDSO support") we see that it starts with the new Makefile approach and doesn't bother with the hard coding from the start. As riscv also started with the new Makefile approach it also could have dropped the hard coding from the start (I guess). > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/kernel/vdso/vdso.lds.S | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S > index 150b1a572e61..7be7e618d59c 100644 > --- a/arch/riscv/kernel/vdso/vdso.lds.S > +++ b/arch/riscv/kernel/vdso/vdso.lds.S > @@ -31,13 +31,7 @@ SECTIONS > > .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) } > > - /* > - * This linker script is used both with -r and with -shared. > - * For the layouts to match, we need to skip more than enough > - * space for the dynamic symbol table, etc. If this amount is > - * insufficient, ld -shared will error; simply increase it here. > - */ > - . = 0x800; > + . = ALIGN(4); I realize 4 is used here now because I questioned the 16, but after doing my digging I think a larger alignment may be better. Loading the text may be done with 8 byte or larger reads, so having the section aligned to a larger size would be better reading it. We might as well use 16, like arm64 does, and like you had before? Also, having enough separation between data and text seems to be important for cache reasons, based on the comment in ./arch/x86/entry/vdso/vdso-layout.lds.S and other vdso history. Maybe we should move .note, .eh_frame_hdr, and .eh_frame below .rodata like x86 has it? Thanks, drew > .text : { *(.text .text.*) } :text > > .data : { > -- > 2.37.2 >
On Thu, Nov 24, 2022 at 11:32:55AM +0100, Andrew Jones wrote: > On Thu, Nov 24, 2022 at 12:18:05AM +0800, Jisheng Zhang wrote: > > It seems the hardcoded 0x800 isn't necessary, but removing it brings a > > s/, but/and/ > > > small vdso.so and aligns with other architectures. > > This commit message didn't really satisfy my desire to understand why > the comment and '. = 0x800' were there in the first place and if its safe > to remove now, so I tried to do some of my own digging. I found > > commit 5b9304933730 ("x86 vDSO: generate vdso-syms.lds") > commit f6b46ebf904f ("x86 vDSO: new layout") > > which removes the comment and hard coding for x86 by changing the vdso > Makefile. Then looking at > > commit 9031fefde6f2 ("arm64: VDSO support") > > we see that it starts with the new Makefile approach and doesn't bother > with the hard coding from the start. As riscv also started with the new > Makefile approach it also could have dropped the hard coding from the > start (I guess). > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > arch/riscv/kernel/vdso/vdso.lds.S | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S > > index 150b1a572e61..7be7e618d59c 100644 > > --- a/arch/riscv/kernel/vdso/vdso.lds.S > > +++ b/arch/riscv/kernel/vdso/vdso.lds.S > > @@ -31,13 +31,7 @@ SECTIONS > > > > .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) } > > > > - /* > > - * This linker script is used both with -r and with -shared. > > - * For the layouts to match, we need to skip more than enough > > - * space for the dynamic symbol table, etc. If this amount is > > - * insufficient, ld -shared will error; simply increase it here. > > - */ > > - . = 0x800; > > + . = ALIGN(4); > > I realize 4 is used here now because I questioned the 16, but after doing > my digging I think a larger alignment may be better. Loading the text may > be done with 8 byte or larger reads, so having the section aligned to a > larger size would be better reading it. We might as well use 16, like > arm64 does, and like you had before? > > Also, having enough separation between data and text seems to be > important for cache reasons, based on the comment in > ./arch/x86/entry/vdso/vdso-layout.lds.S and other vdso history. > Maybe we should move .note, .eh_frame_hdr, and .eh_frame below > .rodata like x86 has it? > Thank you so much for pointing out the two commits and above separation, new version will be sent out soon.
On Fri, Nov 25, 2022 at 12:11:09AM +0800, Jisheng Zhang wrote: > On Thu, Nov 24, 2022 at 11:32:55AM +0100, Andrew Jones wrote: > > On Thu, Nov 24, 2022 at 12:18:05AM +0800, Jisheng Zhang wrote: > > > It seems the hardcoded 0x800 isn't necessary, but removing it brings a > > > > s/, but/and/ > > > > > small vdso.so and aligns with other architectures. > > > > This commit message didn't really satisfy my desire to understand why > > the comment and '. = 0x800' were there in the first place and if its safe > > to remove now, so I tried to do some of my own digging. I found > > > > commit 5b9304933730 ("x86 vDSO: generate vdso-syms.lds") > > commit f6b46ebf904f ("x86 vDSO: new layout") > > > > which removes the comment and hard coding for x86 by changing the vdso > > Makefile. Then looking at > > > > commit 9031fefde6f2 ("arm64: VDSO support") > > > > we see that it starts with the new Makefile approach and doesn't bother > > with the hard coding from the start. As riscv also started with the new > > Makefile approach it also could have dropped the hard coding from the > > start (I guess). > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > arch/riscv/kernel/vdso/vdso.lds.S | 8 +------- > > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S > > > index 150b1a572e61..7be7e618d59c 100644 > > > --- a/arch/riscv/kernel/vdso/vdso.lds.S > > > +++ b/arch/riscv/kernel/vdso/vdso.lds.S > > > @@ -31,13 +31,7 @@ SECTIONS > > > > > > .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) } > > > > > > - /* > > > - * This linker script is used both with -r and with -shared. > > > - * For the layouts to match, we need to skip more than enough > > > - * space for the dynamic symbol table, etc. If this amount is > > > - * insufficient, ld -shared will error; simply increase it here. > > > - */ > > > - . = 0x800; > > > + . = ALIGN(4); > > > > I realize 4 is used here now because I questioned the 16, but after doing > > my digging I think a larger alignment may be better. Loading the text may > > be done with 8 byte or larger reads, so having the section aligned to a > > larger size would be better reading it. We might as well use 16, like > > arm64 does, and like you had before? > > > > Also, having enough separation between data and text seems to be > > important for cache reasons, based on the comment in > > ./arch/x86/entry/vdso/vdso-layout.lds.S and other vdso history. > > Maybe we should move .note, .eh_frame_hdr, and .eh_frame below > > .rodata like x86 has it? > > > > Thank you so much for pointing out the two commits and above > separation, new version will be sent out soon. After looking at the git history, besides your wonderful suggestion, I think we can do something more to clean up and make riscv vdso better, for example, Is the .data section read only, so could be merged with .rodata? or we can discard it as arm64 does? I need some time to dump the section then check and do some test.
On Thu, 24 Nov 2022 08:11:05 PST (-0800), jszhang@kernel.org wrote: > On Thu, Nov 24, 2022 at 11:32:55AM +0100, Andrew Jones wrote: >> On Thu, Nov 24, 2022 at 12:18:05AM +0800, Jisheng Zhang wrote: >> > It seems the hardcoded 0x800 isn't necessary, but removing it brings a >> >> s/, but/and/ >> >> > small vdso.so and aligns with other architectures. >> >> This commit message didn't really satisfy my desire to understand why >> the comment and '. = 0x800' were there in the first place and if its safe >> to remove now, so I tried to do some of my own digging. I found >> >> commit 5b9304933730 ("x86 vDSO: generate vdso-syms.lds") >> commit f6b46ebf904f ("x86 vDSO: new layout") >> >> which removes the comment and hard coding for x86 by changing the vdso >> Makefile. Then looking at >> >> commit 9031fefde6f2 ("arm64: VDSO support") >> >> we see that it starts with the new Makefile approach and doesn't bother >> with the hard coding from the start. As riscv also started with the new >> Makefile approach it also could have dropped the hard coding from the >> start (I guess). >> >> > >> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >> > --- >> > arch/riscv/kernel/vdso/vdso.lds.S | 8 +------- >> > 1 file changed, 1 insertion(+), 7 deletions(-) >> > >> > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S >> > index 150b1a572e61..7be7e618d59c 100644 >> > --- a/arch/riscv/kernel/vdso/vdso.lds.S >> > +++ b/arch/riscv/kernel/vdso/vdso.lds.S >> > @@ -31,13 +31,7 @@ SECTIONS >> > >> > .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) } >> > >> > - /* >> > - * This linker script is used both with -r and with -shared. >> > - * For the layouts to match, we need to skip more than enough >> > - * space for the dynamic symbol table, etc. If this amount is >> > - * insufficient, ld -shared will error; simply increase it here. >> > - */ >> > - . = 0x800; >> > + . = ALIGN(4); >> >> I realize 4 is used here now because I questioned the 16, but after doing >> my digging I think a larger alignment may be better. Loading the text may >> be done with 8 byte or larger reads, so having the section aligned to a >> larger size would be better reading it. We might as well use 16, like >> arm64 does, and like you had before? >> >> Also, having enough separation between data and text seems to be >> important for cache reasons, based on the comment in >> ./arch/x86/entry/vdso/vdso-layout.lds.S and other vdso history. >> Maybe we should move .note, .eh_frame_hdr, and .eh_frame below >> .rodata like x86 has it? >> > > Thank you so much for pointing out the two commits and above > separation, new version will be sent out soon. Not sure if I missed the v2? I can't find one.
On Fri, Dec 09, 2022 at 11:02:18AM -0800, Palmer Dabbelt wrote: > On Thu, 24 Nov 2022 08:11:05 PST (-0800), jszhang@kernel.org wrote: > > On Thu, Nov 24, 2022 at 11:32:55AM +0100, Andrew Jones wrote: > > > On Thu, Nov 24, 2022 at 12:18:05AM +0800, Jisheng Zhang wrote: > > > > It seems the hardcoded 0x800 isn't necessary, but removing it brings a > > > > > > s/, but/and/ > > > > > > > small vdso.so and aligns with other architectures. > > > > > > This commit message didn't really satisfy my desire to understand why > > > the comment and '. = 0x800' were there in the first place and if its safe > > > to remove now, so I tried to do some of my own digging. I found > > > > > > commit 5b9304933730 ("x86 vDSO: generate vdso-syms.lds") > > > commit f6b46ebf904f ("x86 vDSO: new layout") > > > > > > which removes the comment and hard coding for x86 by changing the vdso > > > Makefile. Then looking at > > > > > > commit 9031fefde6f2 ("arm64: VDSO support") > > > > > > we see that it starts with the new Makefile approach and doesn't bother > > > with the hard coding from the start. As riscv also started with the new > > > Makefile approach it also could have dropped the hard coding from the > > > start (I guess). > > > > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > > --- > > > > arch/riscv/kernel/vdso/vdso.lds.S | 8 +------- > > > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > > > > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S > > > > index 150b1a572e61..7be7e618d59c 100644 > > > > --- a/arch/riscv/kernel/vdso/vdso.lds.S > > > > +++ b/arch/riscv/kernel/vdso/vdso.lds.S > > > > @@ -31,13 +31,7 @@ SECTIONS > > > > > > > > .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) } > > > > > > > > - /* > > > > - * This linker script is used both with -r and with -shared. > > > > - * For the layouts to match, we need to skip more than enough > > > > - * space for the dynamic symbol table, etc. If this amount is > > > > - * insufficient, ld -shared will error; simply increase it here. > > > > - */ > > > > - . = 0x800; > > > > + . = ALIGN(4); > > > > > > I realize 4 is used here now because I questioned the 16, but after doing > > > my digging I think a larger alignment may be better. Loading the text may > > > be done with 8 byte or larger reads, so having the section aligned to a > > > larger size would be better reading it. We might as well use 16, like > > > arm64 does, and like you had before? > > > > > > Also, having enough separation between data and text seems to be > > > important for cache reasons, based on the comment in > > > ./arch/x86/entry/vdso/vdso-layout.lds.S and other vdso history. > > > Maybe we should move .note, .eh_frame_hdr, and .eh_frame below > > > .rodata like x86 has it? > > > > > > > Thank you so much for pointing out the two commits and above > > separation, new version will be sent out soon. > > Not sure if I missed the v2? I can't find one. When I want to sent a v2, I just knew that patches after rc7 won't be merged for next rc1. Sorry for not being earlier. Will send out v2 once new developement window is reopen. Thanks
diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S index 150b1a572e61..7be7e618d59c 100644 --- a/arch/riscv/kernel/vdso/vdso.lds.S +++ b/arch/riscv/kernel/vdso/vdso.lds.S @@ -31,13 +31,7 @@ SECTIONS .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) } - /* - * This linker script is used both with -r and with -shared. - * For the layouts to match, we need to skip more than enough - * space for the dynamic symbol table, etc. If this amount is - * insufficient, ld -shared will error; simply increase it here. - */ - . = 0x800; + . = ALIGN(4); .text : { *(.text .text.*) } :text .data : {