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
| ||
|
Message-ID: <20231102140410.ohfp2r7yiuw5gsps@skbuf> Date: Thu, 2 Nov 2023 16:04:10 +0200 From: Vladimir Oltean <olteanv@...il.com> To: Luiz Angelo Daros de Luca <luizluca@...il.com> Cc: netdev@...r.kernel.org, linus.walleij@...aro.org, alsi@...g-olufsen.dk, andrew@...n.ch, vivien.didelot@...il.com, f.fainelli@...il.com, davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, robh+dt@...nel.org, krzk+dt@...nel.org, arinc.unal@...nc9.com Subject: Re: [PATCH net-next v2 3/3] net: dsa: realtek: support reset controller On Wed, Nov 01, 2023 at 04:55:19PM -0300, Luiz Angelo Daros de Luca wrote: > Hi Vladimir, > > > realtek-mdio is an MDIO driver while realtek-smi is a platform driver > > implementing a bitbang protocol. They might never be used together in > > a system to share RAM and not even installed together in small > > systems. If I do need to share the code, I would just link it twice. > > Would something like this be acceptable? > > > > drivers/net/dsa/realtek/Makefile > > -obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > > -obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > > +obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o realtek_common.o > > +obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o realtek_common.o You cannot link realtek_common.o into multiple .ko files. It generates build warnings and it is being phased out. https://patchwork.kernel.org/project/netdevbpf/cover/20221119225650.1044591-1-alobakin@pm.me/ > Just a follow up. > > It is not that simple to include a .c file into an existing single > file module. It looks like you need to rename the original file as all > linked objects must not conflict with the module name. The kernel > build seems to create a new object file for each module. Is there a > clearer way? I think #include a common .c file would not be > acceptable. > > I tested your shared module suggestion. It is the clearest one but it > also increased the overall size quite a bit. Even linking two objects > seems to use the double of space used by the functions alone. These > are some values (mips) > > drivers/net/dsa/realtek/realtek_common.o 5744 without exports > drivers/net/dsa/realtek/realtek_common.o 33472 exporting the two reset functions (assert/deassert) > > drivers/net/dsa/realtek/realtek-mdio.o 18756 without the reset funcs (to be used as a module) > drivers/net/dsa/realtek/realtek-mdio.o 20480 including the realtek_common.c (#include <realtek_common.c>) > drivers/net/dsa/realtek/realtek-mdio.o 22696 linking the realtek_common.o > > drivers/net/dsa/realtek/realtek-smi.o 30712 without the reset funcs (to be used as a module) > drivers/net/dsa/realtek/realtek-smi.o 34604 linking the realtek_common.o > > drivers/net/dsa/realtek/realtek-mdio.ko 28800 without the reset funcs (it will use realtek_common.ko) > drivers/net/dsa/realtek/realtek-mdio.ko 32736 linking the realtek_common.o > > drivers/net/dsa/realtek/realtek-smi.ko 40708 without the reset funcs (it will use realtek_common.ko) > drivers/net/dsa/realtek/realtek-smi.ko 44612 linking the realtek_common.o > > In summary, we get about 1.5kb of code with the extra functions, > almost 4kb if we link a common object containing the functions and > 33kb if we use a module for those two functions. > > I can go with any option. I just need to know which one would be > accepted to update my patches. > 1) keep duplicated functions on each file Disadvantage: need to update the same functions twice, it becomes possible for them to diverge, increases maintenance difficulty. > 2) share the code including the .c on both Including a .c file with a preprocessor #include is fragile, has to be placed very carefully, etc. So it is also a time bomb from a maintenance PoV. Maybe a header with static inline function definitions would be worth considering, although I don't think it's common practice to do this. > 3) share the code linking a common object in both modules (and > renaming existing .c files) Sharing object files is being phased out. > 4) create a new module used by both modules. I am suspicious of the numbers you provided above. What needs to be compared is the reduction in size of realtek_mdio.ko and realtek_smi.ko, compared to the size of the new realtek_common.ko. Also, this starts being more and more worthwhile as more code goes into realtek_common.ko, and I also mentioned a common probe/remove/shutdown as being viable candidates. Looking at your figures, I'm not sure at which ones I need to look in order to compute that metric. > The devices that would use this driver have very restricted storage > space. Every kbyte counts. Well, in that case you could compile the code as built into the kernel, and the module overhead would go away.
Powered by blists - more mailing lists