Message-ID: <20161201135146.295868f0@roar.ozlabs.ibm.com>
Date:   Thu, 1 Dec 2016 13:51:46 +1100
From:   Nicholas Piggin <npiggin@...il.com>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     "H. Peter Anvin" <hpa@...or.com>, Michael Matz <matz@...e.de>,
        Arnd Bergmann <arnd@...db.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Fengguang Wu <fengguang.wu@...el.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        David Ahern <dsahern@...il.com>, Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Wang Nan <wangnan0@...wei.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Borislav Petkov <bp@...en8.de>, Joerg Roedel <joro@...tes.org>
Subject: Re: linker-tables v5 testing

On Wed, 30 Nov 2016 18:38:16 +0100
"Luis R. Rodriguez" <mcgrof@...nel.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@...nel.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

Powered by blists - more mailing lists