On 21 Mar 2023, Luis Chamberlain spake thusly: > On Tue, Mar 21, 2023 at 06:03:01PM +0900, Masahiro Yamada wrote: >> On Tue, Mar 21, 2023 at 6:04 AM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote: >> > In general we could all benefit from an enhancement for a shortname for >> > things which could be modules being built-in. We're now seeing requests >> > for dynamic debug, but it could also be usefulf for Nick's future work >> > to help userspace tools / tracing map kallsysms to specific modules when >> > built-in. >> >> I think I rejected it some years ago. > > Good to know. Indeed! It was rejected because it used too much space in the kernel at runtime (it really did, it was like a megabyte, it was ridiculous). The new approach consumes only about 12K even in a make allyesconfig kernel. Any realistic kernel has much lower overhead, usually about 4KiB total and often even less, but once we're below a page I doubt anyone cares much any more. Load two modules and you've lost more RAM on pure page-granularity slack than that. (This is true to the point where I stopped trying to optimize it further because all the data to determine which module and object file every symbol comes from, to sufficient accuracy to make it possible for every symbol to have a unique (name, module, objname) triplet, was becoming smaller than the size of the code needed to read it!) >> He comes back again and again with almost the same approaches, >> until he finds a "sponsor" (it's you) who will get it in. > > Actually I also rejected the same approach too. ... and that approach is indeed gone, surviving only in a patch series serving as a *validator* to ensure that Luis's approach for tracking built-in modules gives the right result in every case. The validator patch series hasn't been upstreamed. I've given up on that because whenever I tried to upstream it people seem to have assumed that it was incurring costs at build time or in some way being used by the kallsyms code at runtime. It doesn't, it isn't, it's purely a validator, it isn't even executed unless you do make tristatecheck. But not upstreaming it isn't too problematic: even out of tree it still serves its purpose. It's only a developer tool after all. >> When "modprobe foo" succeeds, the user is sure that the kernel >> provides the feature "foo" (but he does not care if >> "foo" is built-in or modular). > > You are thinking about the modprobe situation. That is in no way shape > or form the end goal. Nick suggests he has tooling enhancements which > allow userspace to disambiguate symbols from kallsyms which are built-in > to come things which are modules. DTrace has had this for about a decade now, using a much older and less capable version of the kallmodsyms patch series. I'm working on adding the same sort of thing to perf and ftrace right now. systemtap can already do it, using hacks that would make your hair turn white: hopefully it can move to using /proc/kallmodsyms in time :) > His hacks bring back the tristate crap > which both you and I have rejected. Nah, those are gone and will not come back (see above). In hindsight I actually have a good reason for rejecting them -- they relied on uppercasing CONFIG_ symbols to a y/n value, but a lot of the makefiles contain code that relies on those symbols' values being lowercase! So doing the modules.builtin construction that way leads to wrong results unless apply rather a large pile of tweaks to ugly hacks in makefiles across the tree, and the errors are silent (as witness the fact that nobody had ever fixed it in over a decade). The new approach avoids those errors and gives results precisely matching the tristate values in Kconfig, *as long as* .modinfo is present only in things that can be built as modules. Hence the validator. (Also the makefile trickery in the old scripts/Makefile.modbuiltin is just *unreadable*.) > Having something which could never be a module still use > MODULE_LICENSE() and have modprobe succeed is not fatal. Actually I have since found code in dracut that relies on modprobe failing only if a module it tries to load genuinely does not exist, and succeeding if it's built-in. There is probably more. It seems like something scripts loading modules would be likely to care about. > However, if > future tooling *is* going to be relied upon to display to tracer / > debuggers where symbols come from, it becomes more useful if that > information is a bit more deterministic. I'd say it's even more valuable to use it for tracers/debuggers to arrange to only *trace* things that you want to: so this is input side, deciding what to trace, as much as it is output side in trace/stackwalk output etc. Things like locking primitives are often traceable, but there are many duplicates all with the same names because they're static functions in headers, and you really don't want to trace all the duplicates, only the specific instances you're interested in. (If you trace all of them you likely cause an overwhelming flood of output and possibly crash the kernel due to tracing some bit of locking that it was critical nobody ever trace, which was almost certainly not the specific instance you were concerned with). Function specialization and hot/cold partitioning makes this even worse. > Part of the issue with Nick's patches all along has been proper > justification / documentation. Try to take a step back and think about > the possible *value-add* of userspace having more concrete information > for traces / debugging. ... I hope what I wrote above helps a bit? I'm about to take another look at the cover letter and commit logs for the whole kallmodsyms side of things and try (again) to improve on them.
