On Mon, Jan 9, 2023 at 5:54 AM Nick Alcock nick.alcock@oracle.com wrote:
On 20 Dec 2022, Luis Chamberlain uttered the following:
It also raises the question how many modules have device tables, but do not call MODULE_DEVICE_TABLE since they are only ever built-in. Maybe there should be some build time enforcement mechanism to make sure that these are consistent.
Definitely, Nick Alcock is doing some related work where the semantics of built-in modules needs to be clearer, he for instance is now removing a few MODULE_() macros for things which *are never* modules, and this is because after commit 8b41fc4454e ("kbuild: create modules.builtin without Makefile.modbuiltin or tristate.conf") we rely on the module license tag to generate the modules.builtin file. Without that commit we end up traversing the source tree twice. Nick's work builds on that work and futher clarifies these semantics by adding tooling which complains when something which is *never* capable of being a module uses module macros. The macro you are extending, MODULE_DEVICE_TABLE(), today is a no-op for built-in, but you are adding support to extend it for built-in stuff. Nick's work will help with clarifying symbol locality and so he may be interested in your association for the data in MODULE_DEVICE_TABLE and how you associate to a respective would-be module. His work is useful for making tracing more accurate with respect to symbol associations, so the data in MODULE_DEVICE_TABLE() may be useful as well to him.
The kallmodsyms module info (and, thus, modules.builtin) and MODULE_DEVICE_TABLE do seem interestingly related. I wonder if we can in future reuse at least the module names so we can save a few KiB more space... (in this case, the canonical copy should probably be the one in kallmodsyms, because that lets kallmodsyms reuse strings where modules and their source file have similar names. Something for the future...)
It appeared to me like the symbols added for MODULE_DEVICE_TABLE are only needed temporarily and could be stripped as part of the final linking step. This would make space less of a concern, but extern variables don't support the visibility attribute and in the build I am using the space difference is less than 1MB out of 613MB for the uncompressed kernel.
You folks may want to Cc each other on your patches.
I'd welcome that.
btw, do you want another kallmodsyms patch series from me just arranging to drop fewer MODULE_ entries from non-modules (just MODULE_LICENSE) or would this be considered noise for now? (Are we deadlocked on each other, or are you still looking at the last series I sent, which I think was v10 in late November?)
For now I just need MODULE_DEVICE_TABLE to stick around for USB and thunderbolt related modules (including built-in modules), so if you aren't removing it for any then I don't think we are blocking each other.
Longer term it makes sense to have MODULE_DEVICE_TABLE for any module that makes use of a subsystem that had the authorized attribute. While this is currently just USB/thunderbolt it could expand in the future, but there are subsystems where it is likely to make no difference.
We might have a tiny amount of redundancy in our patch sets because there are some cases of invalid MODULE_DEVICE_TABLE entries I fixed in my patch series, but that could be dropped. These have the potential for conflicts / blocking each other, but it should be easy to resolve them if I change my fixes to a removal of the MODULE_DEVICE_TABLE entries.
-- NULL && (void)