On Wed, 30 Nov 2016 02:33:49 +0100 "Luis R. Rodriguez" <mcg...@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