* [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr
@ 2022-11-23 16:18 Jisheng Zhang
2022-11-24 10:32 ` Andrew Jones
0 siblings, 1 reply; 6+ messages in thread
From: Jisheng Zhang @ 2022-11-23 16:18 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: linux-riscv, linux-kernel, Andrew Jones
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(-)
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 : {
--
2.37.2
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr
2022-11-23 16:18 [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr Jisheng Zhang
@ 2022-11-24 10:32 ` Andrew Jones
2022-11-24 16:11 ` Jisheng Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2022-11-24 10:32 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
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
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr
2022-11-24 10:32 ` Andrew Jones
@ 2022-11-24 16:11 ` Jisheng Zhang
2022-11-24 16:43 ` Jisheng Zhang
2022-12-09 19:02 ` Palmer Dabbelt
0 siblings, 2 replies; 6+ messages in thread
From: Jisheng Zhang @ 2022-11-24 16:11 UTC (permalink / raw)
To: Andrew Jones
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
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.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr
2022-11-24 16:11 ` Jisheng Zhang
@ 2022-11-24 16:43 ` Jisheng Zhang
2022-12-09 19:02 ` Palmer Dabbelt
1 sibling, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2022-11-24 16:43 UTC (permalink / raw)
To: Andrew Jones
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
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.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr
2022-11-24 16:11 ` Jisheng Zhang
2022-11-24 16:43 ` Jisheng Zhang
@ 2022-12-09 19:02 ` Palmer Dabbelt
2022-12-12 15:21 ` Jisheng Zhang
1 sibling, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2022-12-09 19:02 UTC (permalink / raw)
To: jszhang; +Cc: ajones, Paul Walmsley, aou, linux-riscv, linux-kernel
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.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr
2022-12-09 19:02 ` Palmer Dabbelt
@ 2022-12-12 15:21 ` Jisheng Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2022-12-12 15:21 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: ajones, Paul Walmsley, aou, linux-riscv, linux-kernel
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-12 15:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 16:18 [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr Jisheng Zhang
2022-11-24 10:32 ` Andrew Jones
2022-11-24 16:11 ` Jisheng Zhang
2022-11-24 16:43 ` Jisheng Zhang
2022-12-09 19:02 ` Palmer Dabbelt
2022-12-12 15:21 ` Jisheng Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).