linux-kernel - Re: [RFC 0/2] Kbuild: Support nested composite objects

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6837671.QJadu78ljV@zen.local>
Date:   Sun, 13 Jun 2021 09:19:56 -0700
From:   Trent Piepho <tpiepho@...il.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Elliot Berman <eberman@...eaurora.org>
Cc:     Elliot Berman <eberman@...eaurora.org>,
        linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michal Marek <michal.lkml@...kovi.net>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Trilok Soni <tsoni@...eaurora.org>,
        Mahesh Kumar Kalikot Veetil <mkalikot@...eaurora.org>,
        Jeff Johnson <jjohnson@...eaurora.org>
Subject: Re: [RFC 0/2] Kbuild: Support nested composite objects

On Friday, January 22, 2021 11:27:16 AM PDT Elliot Berman wrote:
> The motivation for this series is an out-of-tree module which contains a
> large number of source files. This causes Kbuild to exceed the maximum
> command line argument length when linking the files. Proposal here

I think I am familiar with this module.

In addition to exceeding the argument length when linking, it will fail to 
compile due to 169 include directories being added to c_flags.  Depending on 
root path, this can also overflow the single argument limit of 128 kB.  
Perhaps you have not triggered this yet?  These patches here will not help 
with this issue.  But see below for correct and incorrect fixes.

The fundamental problem is that the kernel build system is designed to be 
recursive, with one makefile per directory.  This module has a single Kbuild 
of over 3000 lines for the entire tree.

Some of this is from this driver not using multiple modules for what is a 
giant driver (337 MB .ko file).  It should be split into multiple modules, if 
only to manage complexity by creating public vs private interfaces through 
choosing what symbols to export or not.  It would avoid merge conflicts on 
the single shared Kbuild file used tree-wide.  Potential name collisions 
across the 100+ include directories could be avoided by not having a shared 
set of include flags for the entire tree.

But some I think is also a limitation of kbuild.  Giant monolithic Kbuilds 
do not work well, but this is ok, because you are supposed to use a 
recursive make design with a Kbuild per directory.  But only non-module code 
may be split across directories.  This important design feature is not 
extended to modules.  A module must be be contained in one directory or 
monolithic tree.  Why should non-module code be allowed the complexity of a 
recursive build but code for a module is not?

I did split this driver.  I'm not aware of any way for me to send patches to 
Qualcomm.   I'd have to maintain this change myself and that would be very 
difficult.

So I worked around it without needing to modify the kernel build system and 
with a minimal patch to Qualcomm's code for me to maintain.

1st, use @file to inject the include directories.  I didn't test well, but I 
think this does not break kbuild's ability to detect changes in command 
lines.  

-ccflags-y += $(INCS)
+ccflags-y += @$(obj)/includes
+$(obj)/includes: FORCE
+       $(call if_changed,include_flag_file)
+
+quiet_cmd_include_flag_file = I-FLAGS $@
+cmd_include_flag_file = echo "$(INCS)" > $@
+
+# To make all objects depend on the file that contains the flags
+$(addprefix $(obj)/,$(OBJS)): $(obj)/includes

The last line might be a bit hacky.  I couldn't find any proper mechanism to 
manually add a dependency to object files.  It seems like auto dependency 
discovered headers are the only ones supported.

Next, we must deal with ~550 object files being linked at once.  The subject 
of this patch series.

-$(MODNAME)-y := $(OBJS)
+$(MODNAME)-y := blob1.o blob2.o blob3.o
+blob1-objs := $(wordlist 1, 200, $(OBJS))
+blob2-objs := $(wordlist 201, 400, $(OBJS))
+blob3-objs := $(wordlist 401, 700, $(OBJS))
+# Multiple levels of composite objects don't really work.  There is no rule
+# to create blobX.o unless we add them to the obj-m list, even though
+# wlan-y lists them and will be used to create wlan.o from them.  We'll get
+# blobX.ko modules we don't actually want by doing this.
+obj-$(CONFIG_QCA_CLD_WLAN) += blob1.o blob2.o blob3.o

The last line here is a total hack, but it does work.  By added blob1.ko as 
a module to be built, a rule to build blob1.o is also created, which will 
then be used by wlan.o's dependency on blob1.o