linker-tables v5 testing

linker-tables v5 testing

[Luis R. Rodriguez]

Guenter,

I think I'm ready to start pushing a new patch set out for review.
Before I do that -- can I trouble you for letting your test
infrastructure hammer it? I'll only push out the patches for review
based on this new set of changes once all tests come back OK for all
architectures.

https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5

Fenguang & Guenter,

Any chance I can trouble you to enable the new driver:
CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
test driver which will WARN_ON() if it finds any errors.

If these warns are captured by your logs then we will see run time use
issues of using this on any architecture for *all* the sections for
linker tables. I had not bothered yet adding a test driver for section
ranges given I already tested
./tools/testing/selftests/ftrace/ftracetest script and confirm things
are still a go.

To the rest on Cc:

Other than a few documentation improvements I think this is now
done... Of course there may be more bike-shedding or few minor
adjustments I may have missed -- Nickolas (or others), please do poke
me with any last minute blockers you see before I push out a new patch
set. I figure there may be some time before the tests are over and I
could probably adjust the series to account for minor things that are
eye-sores before requiring a full new submission.

Also I have a draft edit of a paper on this (hasn't been adjusted to
account for the new API changes yet), if you're in the US and you want
to be anti-social and read something during turkey day perhaps that
might help.

http://drvbp1.linux-foundation.org/~mcgrof/papers/2016/11/21/linker-tables-20161121.pdf

Oh and tools folks:

cd tools/linker-tables/
make clean
make
./demo

  Luis

Re: linker-tables v5 testing

[Guenter Roeck]

On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:
> Guenter,
>
> I think I'm ready to start pushing a new patch set out for review.
> Before I do that -- can I trouble you for letting your test
> infrastructure hammer it? I'll only push out the patches for review

Pushed into my testing branch

> based on this new set of changes once all tests come back OK for all
> architectures.
>
> https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
>
> Fenguang & Guenter,
>
> Any chance I can trouble you to enable the new driver:
> CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
> test driver which will WARN_ON() if it finds any errors.
>
I added this to all defconfigs in the testing branch. That doesn't test
all configurations, but at least those which use a standard defconfig
as base, which by now is most of them.

Guenter

> If these warns are captured by your logs then we will see run time use
> issues of using this on any architecture for *all* the sections for
> linker tables. I had not bothered yet adding a test driver for section
> ranges given I already tested
> ./tools/testing/selftests/ftrace/ftracetest script and confirm things
> are still a go.
>
> To the rest on Cc:
>
> Other than a few documentation improvements I think this is now
> done... Of course there may be more bike-shedding or few minor
> adjustments I may have missed -- Nickolas (or others), please do poke
> me with any last minute blockers you see before I push out a new patch
> set. I figure there may be some time before the tests are over and I
> could probably adjust the series to account for minor things that are
> eye-sores before requiring a full new submission.
>
> Also I have a draft edit of a paper on this (hasn't been adjusted to
> account for the new API changes yet), if you're in the US and you want
> to be anti-social and read something during turkey day perhaps that
> might help.
>
> http://drvbp1.linux-foundation.org/~mcgrof/papers/2016/11/21/linker-tables-20161121.pdf
>
> Oh and tools folks:
>
> cd tools/linker-tables/
> make clean
> make
> ./demo
>
>   Luis
>

Re: linker-tables v5 testing

[Guenter Roeck]

Hi Luis,

On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:
> Guenter,
>
> I think I'm ready to start pushing a new patch set out for review.
> Before I do that -- can I trouble you for letting your test
> infrastructure hammer it? I'll only push out the patches for review
> based on this new set of changes once all tests come back OK for all
> architectures.
>
> https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
>
> Fenguang & Guenter,
>
> Any chance I can trouble you to enable the new driver:
> CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
> test driver which will WARN_ON() if it finds any errors.
>

I see a number of compile failures as well as some crashes in your test driver.
Please have a look. http://kerneltests.org/builders, column 'testing'.

Thanks,
Guenter

Re: linker-tables v5 testing

[Luis R. Rodriguez]

On Thu, Nov 24, 2016 at 08:18:40AM -0800, Guenter Roeck wrote:
> Hi Luis,
> 
> On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:
> > Guenter,
> > 
> > I think I'm ready to start pushing a new patch set out for review.
> > Before I do that -- can I trouble you for letting your test
> > infrastructure hammer it? I'll only push out the patches for review
> > based on this new set of changes once all tests come back OK for all
> > architectures.
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
> > 
> > Fenguang & Guenter,
> > 
> > Any chance I can trouble you to enable the new driver:
> > CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
> > test driver which will WARN_ON() if it finds any errors.
> > 
> 
> I see a number of compile failures as well as some crashes in your test driver.
> Please have a look. http://kerneltests.org/builders, column 'testing'.
> 

Thanks! I believe I have addressed most issues.. Any chance I can have you re-try with
this new branch:

https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5-try2

  Luis

Re: linker-tables v5 testing

[Nicholas Piggin]

On Wed, 30 Nov 2016 02:33:49 +0100
"Luis R. Rodriguez" <mcgrof@kernel.org> wrote:

> On Thu, Nov 24, 2016 at 08:18:40AM -0800, Guenter Roeck wrote:
> > Hi Luis,
> > 
> > On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:  
> > > Guenter,
> > > 
> > > I think I'm ready to start pushing a new patch set out for review.
> > > Before I do that -- can I trouble you for letting your test
> > > infrastructure hammer it? I'll only push out the patches for review
> > > based on this new set of changes once all tests come back OK for all
> > > architectures.
> > > 
> > > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
> > > 
> > > Fenguang & Guenter,
> > > 
> > > Any chance I can trouble you to enable the new driver:
> > > CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
> > > test driver which will WARN_ON() if it finds any errors.
> > >   
> > 
> > I see a number of compile failures as well as some crashes in your test driver.
> > Please have a look. http://kerneltests.org/builders, column 'testing'.
> >   
> 
> Thanks! I believe I have addressed most issues.. Any chance I can have you re-try with
> this new branch:
> 
> https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5-try2

Few minor things:

- Can module-common.lds be generated with standard cpp_lds_S?

- Breaking up the input section descriptions like that in the linker
  script is not what we want AFAIKS. It forces the linker to put them
  in different locations.

Broader issues:

- I still think calling these "sections" and "de facto Linux sections"
  etc. is a bit confusing, especially because assembler sections are
  also involved. Region, array, blob, anything. Then you get "section
  ranges" and "linker tables". Fundamentally all it is is a linear memory
  allocator for static data which is decentralized in the source code
  (as opposed to centralized which would just be `u8 mydata[size]`).

- It would be nice to just clearly separate the memory allocator function
  from the syntatic sugar on top. Actually I think if the memory allocation
  and access functions are clean enough, you don't need anything more. If
  we have an array of pointers and size, it's trivial C code to iterate over
  it. If it needs to have a set of LINKTABLE accessors over the top of it
  for this use case, then that would seem to be a failure of the underlying
  API, no?

- Is it really important to be able to add new allocators without modifying
  a central file for the linker script? Yes it's a benefit, but is it enough
  to justify the complexity?

Thanks,
Nick

Re: linker-tables v5 testing

[Luis R. Rodriguez]

On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:
> On Wed, 30 Nov 2016 02:33:49 +0100
> "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> 
> > On Thu, Nov 24, 2016 at 08:18:40AM -0800, Guenter Roeck wrote:
> > > Hi Luis,
> > > 
> > > On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:  
> > > > Guenter,
> > > > 
> > > > I think I'm ready to start pushing a new patch set out for review.
> > > > Before I do that -- can I trouble you for letting your test
> > > > infrastructure hammer it? I'll only push out the patches for review
> > > > based on this new set of changes once all tests come back OK for all
> > > > architectures.
> > > > 
> > > > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
> > > > 
> > > > Fenguang & Guenter,
> > > > 
> > > > Any chance I can trouble you to enable the new driver:
> > > > CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
> > > > test driver which will WARN_ON() if it finds any errors.
> > > >   
> > > 
> > > I see a number of compile failures as well as some crashes in your test driver.
> > > Please have a look. http://kerneltests.org/builders, column 'testing'.
> > >   
> > 
> > Thanks! I believe I have addressed most issues.. Any chance I can have you re-try with
> > this new branch:
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5-try2
> 
> Few minor things:
> 
> - Can module-common.lds be generated with standard cpp_lds_S?

I actually no longer have a need to have module-common.lds generated given
I no longer am using macro wrappers or helpers. IMHO it would be still be
useful to have this generated but I think we can do this separately so
I can drop these patches and we can address this after?

> - Breaking up the input section descriptions like that in the linker
>   script is not what we want AFAIKS. It forces the linker to put them
>   in different locations.

What is wrong with that ? Separating linker table and section ranges is
not just cosmetic, it also helps us split out domain specific sections
which reduces errors when mismatch errors happen. For instance if you
end up trying to refer to a section range with a linker table helper you'd
get a section mismatch complaint now. Likewise for the inverse.
We also restrict section ranges much more than linker tables, we don't
for instance provide any APIs to iterate over them for instance.

Lastly I wanted SORT() to be used on these sections and while some have
expressed being OK in even sorting our big .text section I think its not
prudent to do so right away to ensure we avoid any possible regressions
because of it. Because of this its best to keep then separate sections
for these special use APIs.

Now it does have cost of modifying the linker script but that just needs
to happen *once* per each new section we want to add support for. This
series only adds section range support and linker table support for a
few basic sections:

  o .data
  o .rodata
  o .text
  o .init.data
  o .init.text

This should suffice for *most* Linux kernel code hacks.

> Broader issues:
> 
> - I still think calling these "sections" and "de facto Linux sections"
>   etc. is a bit confusing, especially because assembler sections are
>   also involved. Region, array, blob, anything. Then you get "section
>   ranges" and "linker tables". Fundamentally all it is is a linear memory
>   allocator for static data which is decentralized in the source code
>   (as opposed to centralized which would just be `u8 mydata[size]`).

You are right, sorry I had not updated that part of the documentation yet.
I had a big nose dive into the ELF specifications after I wrote that
documentation and as per ELF specs all we have here are sections using
SHT_PROGBITS (@progbits on most architectures, on ARM this is %progbits)
and then we have "Special sections" [0] (one is .init for example). The
SHT_PROGBITS is a section type which can be used on programs to specify
a section is custom and its use is defined by the program itself. Now,
some of the "Special sections" though also have SHT_PROGBITS -- and because
of this it implicates programs can further customize these sections. This
is precisely why we can customize .init further on Linux for instance.

I think just calling them Linux sections suffices then. Thoughts?

BTW please refer to a draft paper [1] I have on section ranges / linker tables
in light of this nose dive of the ELF specs [2]. It summarizes also limitations
of ELF sections which are important for these APIs, for instance it should
the answer about limits on 'order level' size, (the limit of say the digit
postfix say 002 in .data.tbl.foo.002). I had not yet updated the documentation
to reflect all this yet but I can do so now if you feel it would help.
Lastly my slides from Plumbers might be useful too [3].

[0] https://refspecs.linuxbase.org/elf/gabi4+/ch4.sheader.html#special_sections
[1] http://drvbp1.linux-foundation.org/~mcgrof/papers/2016/11/21/linker-tables-20161121.pdf
[2] https://refspecs.linuxbase.org/elf/gabi4+/contents.html
[3] http://linuxplumbersconf.com/2016/ocw//system/presentations/3621/original/SUSE-ELF-plumbers.odp
> 
> - It would be nice to just clearly separate the memory allocator function
>   from the syntatic sugar on top.

Sorry I do not follow. What exactly do you mean by this?

>   Actually I think if the memory allocation
>   and access functions are clean enough, you don't need anything more.

Sorry I still do not follow.

>   If we have an array of pointers and size, it's trivial C code to iterate over
>   it. If it needs to have a set of LINKTABLE accessors over the top of it
>   for this use case, then that would seem to be a failure of the underlying
>   API, no?

Still did not get it.

> - Is it really important to be able to add new allocators without modifying
>   a central file for the linker script? Yes it's a benefit, but is it enough
>   to justify the complexity?

If by allocators you mean the ability to add new entries into sections easily
without having to modify the linker script -- then my answer is:

What if its not just about API wrapper to make this easier but also there might
be more benefits on top of this ? What are the benefits I see as we merge this?
They are:

First the obvious gains:

  o Allows us to just use plain C code to add new custom ELF sections
  o Makes it less error prone to add new sections

Then the not so obvious immediate gains:

  o Helps simplify initialization code
  o Helps optionally avoid code bit-rot (see commit "kbuild: enable option to
    force compile force-obj-y and force-lib-y)
  o Enables link time ordering which can help make an extremely lightweight
    dependency annotations explicit (for this refer to the tools/linker-tables/
    example where I've demo'd simplfying Xen init code, which has caused tons of
    regressions over time due to the lack of explicit dependency annotations due
    to the sloppy Xen PV entry path; this is not going away any time soon)

Not so obvious long term gains:

  o Run time ordering customization are still possible (this is really long
    term but we already have one precedent in the kernel that does memmove()
    on entries on a section per some dependency heuristic.)
  o May help prevent use of dead code by either free'ing / nop'ing, no exec
    or whatever sections which we know should definitely not run.
  o Can easily increase the levels of initialization in the kernel without
    much changes (refer to tools/linker-tables/ for an example init with
    linker tables). I've seen a few cases where we've already run out of space
    to get init right in a few subsystems. This not only allows this but also
    allows subsystems to define their own custom levels of init if they wanted
    for their own things.
  o Synthetic functions: enables custom C / asm functions which could help
    add new functionality (see synth_init_or() on tools/linker-tables/drivers/synth/main.c)
    I've been hinted this can help with RAID6 support
  o Self modifying consts (see ps_shr() on tools/linker-tables/drivers/synth/main.c)

  Luis

Re: linker-tables v5 testing

[Nicholas Piggin]

On Wed, 30 Nov 2016 18:38:16 +0100
"Luis R. Rodriguez" <mcgrof@kernel.org> wrote:

> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:
> > On Wed, 30 Nov 2016 02:33:49 +0100
> > "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> >   
> > > On Thu, Nov 24, 2016 at 08:18:40AM -0800, Guenter Roeck wrote:  
> > > > Hi Luis,
> > > > 
> > > > On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:    
> > > > > Guenter,
> > > > > 
> > > > > I think I'm ready to start pushing a new patch set out for review.
> > > > > Before I do that -- can I trouble you for letting your test
> > > > > infrastructure hammer it? I'll only push out the patches for review
> > > > > based on this new set of changes once all tests come back OK for all
> > > > > architectures.
> > > > > 
> > > > > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
> > > > > 
> > > > > Fenguang & Guenter,
> > > > > 
> > > > > Any chance I can trouble you to enable the new driver:
> > > > > CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
> > > > > test driver which will WARN_ON() if it finds any errors.
> > > > >     
> > > > 
> > > > I see a number of compile failures as well as some crashes in your test driver.
> > > > Please have a look. http://kerneltests.org/builders, column 'testing'.
> > > >     
> > > 
> > > Thanks! I believe I have addressed most issues.. Any chance I can have you re-try with
> > > this new branch:
> > > 
> > > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5-try2  
> > 
> > Few minor things:
> > 
> > - Can module-common.lds be generated with standard cpp_lds_S?  
> 
> I actually no longer have a need to have module-common.lds generated given
> I no longer am using macro wrappers or helpers. IMHO it would be still be
> useful to have this generated but I think we can do this separately so
> I can drop these patches and we can address this after?

Okay.

> 
> > - Breaking up the input section descriptions like that in the linker
> >   script is not what we want AFAIKS. It forces the linker to put them
> >   in different locations.  
> 
> What is wrong with that ? Separating linker table and section ranges is

It's not that you separate those, of course you need that. It's that
you also separate other sections from the input section descriptions:

-		*(.text.hot .text .text.fixup .text.unlikely)		\
+		*(.text.hot .text)					\
+		*(SORT(.text.rng.*))					\
+		*(.text.fixup .text.unlikely)				\

[snip]

> 
> > Broader issues:
> > 
> > - I still think calling these "sections" and "de facto Linux sections"
> >   etc. is a bit confusing, especially because assembler sections are
> >   also involved. Region, array, blob, anything. Then you get "section
> >   ranges" and "linker tables". Fundamentally all it is is a linear memory
> >   allocator for static data which is decentralized in the source code
> >   (as opposed to centralized which would just be `u8 mydata[size]`).  
> 
> You are right, sorry I had not updated that part of the documentation yet.
> I had a big nose dive into the ELF specifications after I wrote that
> documentation and as per ELF specs all we have here are sections using
> SHT_PROGBITS (@progbits on most architectures, on ARM this is %progbits)
> and then we have "Special sections" [0] (one is .init for example). The
> SHT_PROGBITS is a section type which can be used on programs to specify
> a section is custom and its use is defined by the program itself. Now,
> some of the "Special sections" though also have SHT_PROGBITS -- and because
> of this it implicates programs can further customize these sections. This
> is precisely why we can customize .init further on Linux for instance.
> 
> I think just calling them Linux sections suffices then. Thoughts?

I suppose. I don't have a much better idea so I'll end the bikeshedding
it. Oh just one last splash of paint: the other thing is "linker tables".
If you call the general concept a linux section, then can linker tables
be linux section tables, or linux section arrays?

> BTW please refer to a draft paper [1] I have on section ranges / linker tables
> in light of this nose dive of the ELF specs [2]. It summarizes also limitations
> of ELF sections which are important for these APIs, for instance it should
> the answer about limits on 'order level' size, (the limit of say the digit
> postfix say 002 in .data.tbl.foo.002). I had not yet updated the documentation
> to reflect all this yet but I can do so now if you feel it would help.
> Lastly my slides from Plumbers might be useful too [3].
> 
> [0] https://refspecs.linuxbase.org/elf/gabi4+/ch4.sheader.html#special_sections
> [1] http://drvbp1.linux-foundation.org/~mcgrof/papers/2016/11/21/linker-tables-20161121.pdf
> [2] https://refspecs.linuxbase.org/elf/gabi4+/contents.html
> [3] http://linuxplumbersconf.com/2016/ocw//system/presentations/3621/original/SUSE-ELF-plumbers.odp

I'll take a look at them when I get a bit more time.

> > 
> > - It would be nice to just clearly separate the memory allocator function
> >   from the syntatic sugar on top.  
> 
> Sorry I do not follow. What exactly do you mean by this?
> 
> >   Actually I think if the memory allocation
> >   and access functions are clean enough, you don't need anything more.  
> 
> Sorry I still do not follow.
> 
> >   If we have an array of pointers and size, it's trivial C code to iterate over
> >   it. If it needs to have a set of LINKTABLE accessors over the top of it
> >   for this use case, then that would seem to be a failure of the underlying
> >   API, no?  
> 
> Still did not get it.

Well fundamentally the linker table is just a way to declare some type of
array that any code can add some elements to, right? The non-C aspect of it
is this ability of producers to be decentralized.

The consumer is not really different from any other array though. They just
want to know the address and the number of elements, and that's all. You can
provide that with two macros and don't really need macros like for each, run
all, etc because it's just simple C iteration over an array.

> > - Is it really important to be able to add new allocators without modifying
> >   a central file for the linker script? Yes it's a benefit, but is it enough
> >   to justify the complexity?  
> 
> If by allocators you mean the ability to add new entries into sections easily
> without having to modify the linker script -- then my answer is:

Yes, but after reading a little closer it may not really be a problem I
first thought. Thanks for providing the detailed points.

Thanks,
Nick

> 
> What if its not just about API wrapper to make this easier but also there might
> be more benefits on top of this ? What are the benefits I see as we merge this?
> They are:

> 
> First the obvious gains:
> 
>   o Allows us to just use plain C code to add new custom ELF sections
>   o Makes it less error prone to add new sections
> 
> Then the not so obvious immediate gains:
> 
>   o Helps simplify initialization code
>   o Helps optionally avoid code bit-rot (see commit "kbuild: enable option to
>     force compile force-obj-y and force-lib-y)
>   o Enables link time ordering which can help make an extremely lightweight
>     dependency annotations explicit (for this refer to the tools/linker-tables/
>     example where I've demo'd simplfying Xen init code, which has caused tons of
>     regressions over time due to the lack of explicit dependency annotations due
>     to the sloppy Xen PV entry path; this is not going away any time soon)
> 
> Not so obvious long term gains:
> 
>   o Run time ordering customization are still possible (this is really long
>     term but we already have one precedent in the kernel that does memmove()
>     on entries on a section per some dependency heuristic.)
>   o May help prevent use of dead code by either free'ing / nop'ing, no exec
>     or whatever sections which we know should definitely not run.
>   o Can easily increase the levels of initialization in the kernel without
>     much changes (refer to tools/linker-tables/ for an example init with
>     linker tables). I've seen a few cases where we've already run out of space
>     to get init right in a few subsystems. This not only allows this but also
>     allows subsystems to define their own custom levels of init if they wanted
>     for their own things.
>   o Synthetic functions: enables custom C / asm functions which could help
>     add new functionality (see synth_init_or() on tools/linker-tables/drivers/synth/main.c)
>     I've been hinted this can help with RAID6 support
>   o Self modifying consts (see ps_shr() on tools/linker-tables/drivers/synth/main.c)
> 
>   Luis

Re: linker-tables v5 testing

[Luis R. Rodriguez]

On Wed, Nov 30, 2016 at 6:51 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> On Wed, 30 Nov 2016 18:38:16 +0100
> "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>
>> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:
>> I actually no longer have a need to have module-common.lds generated given
>> I no longer am using macro wrappers or helpers. IMHO it would be still be
>> useful to have this generated but I think we can do this separately so
>> I can drop these patches and we can address this after?
>
> Okay.

OK dropped.

>> > - Breaking up the input section descriptions like that in the linker
>> >   script is not what we want AFAIKS. It forces the linker to put them
>> >   in different locations.
>>
>> What is wrong with that ? Separating linker table and section ranges is
>
> It's not that you separate those, of course you need that. It's that
> you also separate other sections from the input section descriptions:
>
> -               *(.text.hot .text .text.fixup .text.unlikely)           \
> +               *(.text.hot .text)                                      \
> +               *(SORT(.text.rng.*))                                    \
> +               *(.text.fixup .text.unlikely)                           \
>
> [snip]

And ?

>> > Broader issues:
>> >
>> > - I still think calling these "sections" and "de facto Linux sections"
>> >   etc. is a bit confusing, especially because assembler sections are
>> >   also involved. Region, array, blob, anything. Then you get "section
>> >   ranges" and "linker tables". Fundamentally all it is is a linear memory
>> >   allocator for static data which is decentralized in the source code
>> >   (as opposed to centralized which would just be `u8 mydata[size]`).
>>
>> You are right, sorry I had not updated that part of the documentation yet.
>> I had a big nose dive into the ELF specifications after I wrote that
>> documentation and as per ELF specs all we have here are sections using
>> SHT_PROGBITS (@progbits on most architectures, on ARM this is %progbits)
>> and then we have "Special sections" [0] (one is .init for example). The
>> SHT_PROGBITS is a section type which can be used on programs to specify
>> a section is custom and its use is defined by the program itself. Now,
>> some of the "Special sections" though also have SHT_PROGBITS -- and because
>> of this it implicates programs can further customize these sections. This
>> is precisely why we can customize .init further on Linux for instance.
>>
>> I think just calling them Linux sections suffices then. Thoughts?
>
> I suppose. I don't have a much better idea so I'll end the bikeshedding
> it. Oh just one last splash of paint: the other thing is "linker tables".
> If you call the general concept a linux section, then can linker tables
> be linux section tables, or linux section arrays?

I stuck with "linker tables" as iPXE was a huge source of inspiration
and that's what they called their solution. What we have is very
different and their purpose is only one option for us now. Sure..
"linker section tables" and "linker section ranges".

>> > - It would be nice to just clearly separate the memory allocator function
>> >   from the syntatic sugar on top.
>>
>> Sorry I do not follow. What exactly do you mean by this?
>>
>> >   Actually I think if the memory allocation
>> >   and access functions are clean enough, you don't need anything more.
>>
>> Sorry I still do not follow.
>>
>> >   If we have an array of pointers and size, it's trivial C code to iterate over
>> >   it. If it needs to have a set of LINKTABLE accessors over the top of it
>> >   for this use case, then that would seem to be a failure of the underlying
>> >   API, no?
>>
>> Still did not get it.
>
> Well fundamentally the linker table is just a way to declare some type of
> array that any code can add some elements to, right?

Well in particular to a special section, and we're providing standard
easy way to do this without any hacky linker script or assembly.

> The non-C aspect of it
> is this ability of producers to be decentralized.

Not sure what you mean here, we only do a little bit of linker table
magic, tweaks and then provide helpers.

> The consumer is not really different from any other array though. They just
> want to know the address and the number of elements, and that's all. You can
> provide that with two macros and don't really need macros like for each, run
> all, etc because it's just simple C iteration over an array.

I added those at the request of hpa, and one of the reasons was that
we tend to historically use these "arrays" in special ways all over
the kernel. But these arrays are very special, they are not typical
arrays. This makes emphasis on its use. They are also needed given the
special formatting we have for start / end of these in a generic form.
This should all help consolidate this and make it easier to use in a
generic form. It should also help avoid mismatch use when the intent
was a section range and we're on a linker table, or the other way
around.

>> > - Is it really important to be able to add new allocators without modifying
>> >   a central file for the linker script? Yes it's a benefit, but is it enough
>> >   to justify the complexity?
>>
>> If by allocators you mean the ability to add new entries into sections easily
>> without having to modify the linker script -- then my answer is:
>
> Yes, but after reading a little closer it may not really be a problem I
> first thought. Thanks for providing the detailed points.

OK. So just a bit of bike shedding ?

BTW the test driver lib/test_linktables/test-linktables.c seems to
fail on ppc at run time at:

rc = trigger_config_run_named(test_dev, ".text");

I'm having issues getting a ppc / ppc64 box going to easily verify and
objdump / test this further. Anyone with a ppc / ppc64 willing to
objdump the output of this driver would be appreciated... the latest
branch is:

https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161130-linker-tables-v5

  Luis

Re: linker-tables v5 testing

[Nicholas Piggin]

On Wed, 30 Nov 2016 19:15:27 -0800
"Luis R. Rodriguez" <mcgrof@kernel.org> wrote:

> On Wed, Nov 30, 2016 at 6:51 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > On Wed, 30 Nov 2016 18:38:16 +0100
> > "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> >  
> >> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:

> >> What is wrong with that ? Separating linker table and section ranges is  
> >
> > It's not that you separate those, of course you need that. It's that
> > you also separate other sections from the input section descriptions:
> >
> > -               *(.text.hot .text .text.fixup .text.unlikely)           \
> > +               *(.text.hot .text)                                      \
> > +               *(SORT(.text.rng.*))                                    \
> > +               *(.text.fixup .text.unlikely)                           \
> >
> > [snip]  
> 
> And ?

Umm.. don't remember :)


> >> >   If we have an array of pointers and size, it's trivial C code to iterate over
> >> >   it. If it needs to have a set of LINKTABLE accessors over the top of it
> >> >   for this use case, then that would seem to be a failure of the underlying
> >> >   API, no?  
> >>
> >> Still did not get it.  
> >
> > Well fundamentally the linker table is just a way to declare some type of
> > array that any code can add some elements to, right?  
> 
> Well in particular to a special section, and we're providing standard
> easy way to do this without any hacky linker script or assembly.

Right.

> > The non-C aspect of it
> > is this ability of producers to be decentralized.  
> 
> Not sure what you mean here, we only do a little bit of linker table
> magic, tweaks and then provide helpers.

I mean that you don't need some LINKTABLE_FOREACH accessor for it,
because from the consumer point of view, it's a simple array. It can
easily be handled by plain C. All we need is to get the array base
and size.


> > The consumer is not really different from any other array though. They just
> > want to know the address and the number of elements, and that's all. You can
> > provide that with two macros and don't really need macros like for each, run
> > all, etc because it's just simple C iteration over an array.  
> 
> I added those at the request of hpa, and one of the reasons was that
> we tend to historically use these "arrays" in special ways all over
> the kernel. But these arrays are very special, they are not typical
> arrays. This makes emphasis on its use. They are also needed given the
> special formatting we have for start / end of these in a generic form.
> This should all help consolidate this and make it easier to use in a
> generic form. It should also help avoid mismatch use when the intent
> was a section range and we're on a linker table, or the other way
> around.

I disagree. Making a simple iterator for array, or hiding a simple
function call behind it, really doesn't buy you anything. It's not
like linked list for example, where the result of the iterators is
much nicer than the open coded C.


> >> > - Is it really important to be able to add new allocators without modifying
> >> >   a central file for the linker script? Yes it's a benefit, but is it enough
> >> >   to justify the complexity?  
> >>
> >> If by allocators you mean the ability to add new entries into sections easily
> >> without having to modify the linker script -- then my answer is:  
> >
> > Yes, but after reading a little closer it may not really be a problem I
> > first thought. Thanks for providing the detailed points.  
> 
> OK. So just a bit of bike shedding ?

No I just misread how a part of it was implemented.

Thanks,
Nick

Re: linker-tables v5 testing

[Nicholas Piggin]

On Thu, 1 Dec 2016 16:04:30 +1100
Nicholas Piggin <npiggin@gmail.com> wrote:

> On Wed, 30 Nov 2016 19:15:27 -0800
> "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> 
> > On Wed, Nov 30, 2016 at 6:51 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > > On Wed, 30 Nov 2016 18:38:16 +0100
> > > "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> > >  
> > >> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:
> 
> > >> What is wrong with that ? Separating linker table and section ranges is  
> > >
> > > It's not that you separate those, of course you need that. It's that
> > > you also separate other sections from the input section descriptions:
> > >
> > > -               *(.text.hot .text .text.fixup .text.unlikely)           \
> > > +               *(.text.hot .text)                                      \
> > > +               *(SORT(.text.rng.*))                                    \
> > > +               *(.text.fixup .text.unlikely)                           \

Ahh, you're doing it to avoid clash with compiler generated sections.
The usual way to cope with that seems to be to use two dots for your name.
.text..rng.*

Re: linker-tables v5 testing

[Luis R. Rodriguez]

On Wed, Nov 30, 2016 at 9:20 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> On Thu, 1 Dec 2016 16:04:30 +1100
> Nicholas Piggin <npiggin@gmail.com> wrote:
>
>> On Wed, 30 Nov 2016 19:15:27 -0800
>> "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>>
>> > On Wed, Nov 30, 2016 at 6:51 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>> > > On Wed, 30 Nov 2016 18:38:16 +0100
>> > > "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>> > >
>> > >> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:
>>
>> > >> What is wrong with that ? Separating linker table and section ranges is
>> > >
>> > > It's not that you separate those, of course you need that. It's that
>> > > you also separate other sections from the input section descriptions:
>> > >
>> > > -               *(.text.hot .text .text.fixup .text.unlikely)           \
>> > > +               *(.text.hot .text)                                      \
>> > > +               *(SORT(.text.rng.*))                                    \
>> > > +               *(.text.fixup .text.unlikely)                           \
>
> Ahh, you're doing it to avoid clash with compiler generated sections.

Nope, its for two reasons:

1) To be able to construct arrays without modifying the linker script
we had to get crafty, and opted in for the trick of picking two
arbitrary delimiters for use of section start, and section end, namely
the tilde character ("~") and the empty string (""), and then stuffing
anything in between. For this to work properly we must SORT() these
specially crafted sections as well.

2) Because I don't want to regress .text if SORT()'ing it breaks
something. In theory it should not but I rather be careful.

> The usual way to cope with that seems to be to use two dots for your name.
> .text..rng.*

I have been wondering why people started doing that, it was not clear
nor documented anywhere. So no, it was not my original motivation, but
if it helps, it will be good to document this as well.

 Luis

Re: linker-tables v5 testing

[Luis R. Rodriguez]

On Thu, Dec 1, 2016 at 10:31 PM, Nicholas Piggin
<nicholas.piggin@gmail.com> wrote:
>
> On 2 Dec 2016 2:35 AM, "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>>
>> On Wed, Nov 30, 2016 at 9:20 PM, Nicholas Piggin <npiggin@gmail.com>
>> wrote:
>> > On Thu, 1 Dec 2016 16:04:30 +1100
>> > Nicholas Piggin <npiggin@gmail.com> wrote:
>> >
>> >> On Wed, 30 Nov 2016 19:15:27 -0800
>> >> "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>> >>
>> >> > On Wed, Nov 30, 2016 at 6:51 PM, Nicholas Piggin <npiggin@gmail.com>
>> >> > wrote:
>> >> > > On Wed, 30 Nov 2016 18:38:16 +0100
>> >> > > "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
>> >> > >
>> >> > >> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:
>> >>
>> >> > >> What is wrong with that ? Separating linker table and section
>> >> > >> ranges is
>> >> > >
>> >> > > It's not that you separate those, of course you need that. It's
>> >> > > that
>> >> > > you also separate other sections from the input section
>> >> > > descriptions:
>> >> > >
>> >> > > -               *(.text.hot .text .text.fixup .text.unlikely)
>> >> > > \
>> >> > > +               *(.text.hot .text)
>> >> > > \
>> >> > > +               *(SORT(.text.rng.*))
>> >> > > \
>> >> > > +               *(.text.fixup .text.unlikely)
>> >> > > \
>> >
>> > Ahh, you're doing it to avoid clash with compiler generated sections.
>>
>> Nope, its for two reasons:
>>
>> 1) To be able to construct arrays without modifying the linker script
>> we had to get crafty, and opted in for the trick of picking two
>> arbitrary delimiters for use of section start, and section end, namely
>> the tilde character ("~") and the empty string (""), and then stuffing
>> anything in between. For this to work properly we must SORT() these
>> specially crafted sections as well.
>>
>> 2) Because I don't want to regress .text if SORT()'ing it breaks
>> something. In theory it should not but I rather be careful.
>
> Oh yes I wasn't talking about the sort itself, but about why you broke up
> the existing input section descriptions.

Ah, but I was also talking about this, the only thing is...

> That was my only concern, e.g.,
> .data and .data.[_0-9a-zA-Z] spoke be in the same description.

I was talking about splitting up .data and both .data.tbl .data.rng --
you were talking specifically only about .data and .data.[_0-9a-zA-Z]
and now I see the concern! Yes, you are -- we can avoid this, its just
your glob would also capture .data.tbl and data.rng and I want to sort
these so I do it first. But as you already deduced, this should still
have no harm as I am just stuffing it in well sorted first and the
later glob just captures that. I could have just used .data..tbl and
data..rng as you noted. Either way is fine by me.

Do you have a preference?

>> > The usual way to cope with that seems to be to use two dots for your
>> > name.
>> > .text..rng.*
>>
>> I have been wondering why people started doing that, it was not clear
>> nor documented anywhere. So no, it was not my original motivation, but
>> if it helps, it will be good to document this as well.
>
> Yeah it's convention because . can be part of a section name but not a C
> symbol.

Great, makes sense -- do you have references for this practice BTW or
did you just infer this?

 Luis

Re: linker-tables v5 testing

[Nicholas Piggin]

On Fri, 2 Dec 2016 07:49:52 -0800
"Luis R. Rodriguez" <mcgrof@kernel.org> wrote:

> On Thu, Dec 1, 2016 at 10:31 PM, Nicholas Piggin
> <nicholas.piggin@gmail.com> wrote:
> >
> > On 2 Dec 2016 2:35 AM, "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:  
> >>
> >> On Wed, Nov 30, 2016 at 9:20 PM, Nicholas Piggin <npiggin@gmail.com>
> >> wrote:  
> >> > On Thu, 1 Dec 2016 16:04:30 +1100
> >> > Nicholas Piggin <npiggin@gmail.com> wrote:
> >> >  
> >> >> On Wed, 30 Nov 2016 19:15:27 -0800
> >> >> "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> >> >>  
> >> >> > On Wed, Nov 30, 2016 at 6:51 PM, Nicholas Piggin <npiggin@gmail.com>
> >> >> > wrote:  
> >> >> > > On Wed, 30 Nov 2016 18:38:16 +0100
> >> >> > > "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> >> >> > >  
> >> >> > >> On Wed, Nov 30, 2016 at 02:09:47PM +1100, Nicholas Piggin wrote:  
> >> >>  
> >> >> > >> What is wrong with that ? Separating linker table and section
> >> >> > >> ranges is  
> >> >> > >
> >> >> > > It's not that you separate those, of course you need that. It's
> >> >> > > that
> >> >> > > you also separate other sections from the input section
> >> >> > > descriptions:
> >> >> > >
> >> >> > > -               *(.text.hot .text .text.fixup .text.unlikely)
> >> >> > > \
> >> >> > > +               *(.text.hot .text)
> >> >> > > \
> >> >> > > +               *(SORT(.text.rng.*))
> >> >> > > \
> >> >> > > +               *(.text.fixup .text.unlikely)
> >> >> > > \  
> >> >
> >> > Ahh, you're doing it to avoid clash with compiler generated sections.  
> >>
> >> Nope, its for two reasons:
> >>
> >> 1) To be able to construct arrays without modifying the linker script
> >> we had to get crafty, and opted in for the trick of picking two
> >> arbitrary delimiters for use of section start, and section end, namely
> >> the tilde character ("~") and the empty string (""), and then stuffing
> >> anything in between. For this to work properly we must SORT() these
> >> specially crafted sections as well.
> >>
> >> 2) Because I don't want to regress .text if SORT()'ing it breaks
> >> something. In theory it should not but I rather be careful.  
> >
> > Oh yes I wasn't talking about the sort itself, but about why you broke up
> > the existing input section descriptions.  
> 
> Ah, but I was also talking about this, the only thing is...
> 
> > That was my only concern, e.g.,
> > .data and .data.[_0-9a-zA-Z] spoke be in the same description.  
> 
> I was talking about splitting up .data and both .data.tbl .data.rng --
> you were talking specifically only about .data and .data.[_0-9a-zA-Z]
> and now I see the concern! Yes, you are -- we can avoid this, its just
> your glob would also capture .data.tbl and data.rng and I want to sort
> these so I do it first. But as you already deduced, this should still
> have no harm as I am just stuffing it in well sorted first and the
> later glob just captures that. I could have just used .data..tbl and
> data..rng as you noted. Either way is fine by me.
> 
> Do you have a preference?

I would say use "..", just so there is less ambiguity and more
flexibility in moving around the input section descriptions if needed.

> 
> >> > The usual way to cope with that seems to be to use two dots for your
> >> > name.
> >> > .text..rng.*  
> >>
> >> I have been wondering why people started doing that, it was not clear
> >> nor documented anywhere. So no, it was not my original motivation, but
> >> if it helps, it will be good to document this as well.  
> >
> > Yeah it's convention because . can be part of a section name but not a C
> > symbol.  
> 
> Great, makes sense -- do you have references for this practice BTW or
> did you just infer this?

I believe I've seen something, perhaps it was in the binutils manuals,
but I tried just now and couldn't find anything.

It is a common practice to use dots in labels to avoid clashes with C
names in general. Specifically for sections we already have some cases
in there which I believe were added for similar reasons.

Thanks,
Nick

Re: linker-tables v5 testing

[Luis R. Rodriguez]

On Wed, Nov 30, 2016 at 7:15 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> BTW the test driver lib/test_linktables/test-linktables.c seems to
> fail on ppc at run time at:
>
> rc = trigger_config_run_named(test_dev, ".text");
>
> I'm having issues getting a ppc / ppc64 box going to easily verify and
> objdump / test this further. Anyone with a ppc / ppc64 willing to
> objdump the output of this driver would be appreciated... the latest
> branch is:
>
> https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161130-linker-tables-v5

FWIW I've fixed ppc for .text -- it does not use TEXT_TEXT so it just
needed the rng and tbl entries added to it.

 Luis

Re: linker-tables v5 testing

[Guenter Roeck]

On 11/29/2016 05:33 PM, Luis R. Rodriguez wrote:
> On Thu, Nov 24, 2016 at 08:18:40AM -0800, Guenter Roeck wrote:
>> Hi Luis,
>>
>> On 11/23/2016 08:11 PM, Luis R. Rodriguez wrote:
>>> Guenter,
>>>
>>> I think I'm ready to start pushing a new patch set out for review.
>>> Before I do that -- can I trouble you for letting your test
>>> infrastructure hammer it? I'll only push out the patches for review
>>> based on this new set of changes once all tests come back OK for all
>>> architectures.
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5
>>>
>>> Fenguang & Guenter,
>>>
>>> Any chance I can trouble you to enable the new driver:
>>> CONFIG_TEST_LINKTABLES=y on each kernel configuration as it will run a
>>> test driver which will WARN_ON() if it finds any errors.
>>>
>>
>> I see a number of compile failures as well as some crashes in your test driver.
>> Please have a look. http://kerneltests.org/builders, column 'testing'.
>>
>
> Thanks! I believe I have addressed most issues.. Any chance I can have you re-try with
> this new branch:
>
> https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161117-linker-tables-v5-try2
>
Pushed ... should start shortly. I'll let you know how it goes.

Guenter