| Message ID | 20240617175818.58219-17-samitolvanen@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Implement MODVERSIONS for Rust | expand |
On Tue, Jun 18, 2024 at 2:58 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > Hi folks, > > This series implements CONFIG_MODVERSIONS for Rust, an important > feature for distributions like Android that want to ship Rust > kernel modules, and depend on modversions to help ensure module ABI > compatibility. > > There have been earlier proposals [1][2] that would allow Rust > modules to coexist with modversions, but none that actually implement > symbol versioning. Unlike C, Rust source code doesn't have sufficient > information about the final ABI, as the compiler has considerable > freedom in adjusting structure layout for improved performance [3], > for example, which makes using a source code parser like genksyms > a non-starter. Based on Matt's suggestion and previous feedback > from maintainers, this series uses DWARF debugging information for > computing versions. DWARF is an established and relatively stable > format, which includes all the necessary ABI details, and adding a > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > reasonable trade-off. > > The first 12 patches of this series add a small tool for computing > symbol versions from DWARF, called gendwarfksyms. When passed a list > of exported symbols, the tool generates an expanded type string > for each symbol, and computes symbol CRCs similarly to genksyms. > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > because of the existing support for C host tools that use elfutils > (e.g., objtool). > > Another compatibility issue is fitting the extremely long mangled > Rust symbol names into struct modversion_info, which can only hold > 55-character names (on 64-bit systems). Previous proposals suggested > changing the structure to support longer names, but the conclusion was > that we cannot break userspace tools that parse the version table. > > The next two patches of the series implement support for hashed > symbol names in struct modversion_info, where names longer than 55 > characters are hashed, and the hash is stored in the name field. To > avoid breaking userspace tools, the binary hash is prefixed with a > null-terminated string containing the name of the hash function. While > userspace tools can later be updated to potentially produce more > useful information about the long symbols, this allows them to > continue working in the meantime. > > The final patch allows CONFIG_MODVERSIONS to be selected with Rust, > provided that debugging information is also available. I am surprised at someone who attempts to add another variant of genksyms. I am also surprised at the tool being added under the tools/ directory. At least, we can avoid the latter misfortune, though. A patch attached (on top of your patch set). Such a tool can be compiled in scripts/, or even better in rust/Makefile if it is only used in rust/Makefile. -- Best Regards Masahiro Yamada
On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > Hi folks, > > This series implements CONFIG_MODVERSIONS for Rust, an important > feature for distributions like Android that want to ship Rust > kernel modules, and depend on modversions to help ensure module ABI > compatibility. > > There have been earlier proposals [1][2] that would allow Rust > modules to coexist with modversions, but none that actually implement > symbol versioning. Unlike C, Rust source code doesn't have sufficient > information about the final ABI, as the compiler has considerable > freedom in adjusting structure layout for improved performance [3], > for example, which makes using a source code parser like genksyms > a non-starter. Based on Matt's suggestion and previous feedback > from maintainers, this series uses DWARF debugging information for > computing versions. DWARF is an established and relatively stable > format, which includes all the necessary ABI details, and adding a > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > reasonable trade-off. > > The first 12 patches of this series add a small tool for computing > symbol versions from DWARF, called gendwarfksyms. When passed a list > of exported symbols, the tool generates an expanded type string > for each symbol, and computes symbol CRCs similarly to genksyms. > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > because of the existing support for C host tools that use elfutils > (e.g., objtool). That's cool, can the C code be switched to also use this? That way we only have one path/code for all of this? thanks, greg k-h
On Wed, Jun 19, 2024 at 1:44 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > > Hi folks, > > > > This series implements CONFIG_MODVERSIONS for Rust, an important > > feature for distributions like Android that want to ship Rust > > kernel modules, and depend on modversions to help ensure module ABI > > compatibility. > > > > There have been earlier proposals [1][2] that would allow Rust > > modules to coexist with modversions, but none that actually implement > > symbol versioning. Unlike C, Rust source code doesn't have sufficient > > information about the final ABI, as the compiler has considerable > > freedom in adjusting structure layout for improved performance [3], > > for example, which makes using a source code parser like genksyms > > a non-starter. Based on Matt's suggestion and previous feedback > > from maintainers, this series uses DWARF debugging information for > > computing versions. DWARF is an established and relatively stable > > format, which includes all the necessary ABI details, and adding a > > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > > reasonable trade-off. > > > > The first 12 patches of this series add a small tool for computing > > symbol versions from DWARF, called gendwarfksyms. When passed a list > > of exported symbols, the tool generates an expanded type string > > for each symbol, and computes symbol CRCs similarly to genksyms. > > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > > because of the existing support for C host tools that use elfutils > > (e.g., objtool). > > That's cool, can the C code be switched to also use this? That way we > only have one path/code for all of this? As the description says, it requires CONFIG_DEBUG_INFO. We can strip the debug info from the final vmlinux, but I guess the build speed will be even slower than the current genksyms.
On Wed, Jun 19, 2024 at 01:50:36AM +0900, Masahiro Yamada wrote: > On Wed, Jun 19, 2024 at 1:44 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > > > Hi folks, > > > > > > This series implements CONFIG_MODVERSIONS for Rust, an important > > > feature for distributions like Android that want to ship Rust > > > kernel modules, and depend on modversions to help ensure module ABI > > > compatibility. > > > > > > There have been earlier proposals [1][2] that would allow Rust > > > modules to coexist with modversions, but none that actually implement > > > symbol versioning. Unlike C, Rust source code doesn't have sufficient > > > information about the final ABI, as the compiler has considerable > > > freedom in adjusting structure layout for improved performance [3], > > > for example, which makes using a source code parser like genksyms > > > a non-starter. Based on Matt's suggestion and previous feedback > > > from maintainers, this series uses DWARF debugging information for > > > computing versions. DWARF is an established and relatively stable > > > format, which includes all the necessary ABI details, and adding a > > > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > > > reasonable trade-off. > > > > > > The first 12 patches of this series add a small tool for computing > > > symbol versions from DWARF, called gendwarfksyms. When passed a list > > > of exported symbols, the tool generates an expanded type string > > > for each symbol, and computes symbol CRCs similarly to genksyms. > > > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > > > because of the existing support for C host tools that use elfutils > > > (e.g., objtool). > > > > That's cool, can the C code be switched to also use this? That way we > > only have one path/code for all of this? > > > As the description says, it requires CONFIG_DEBUG_INFO. > We can strip the debug info from the final vmlinux, but > I guess the build speed will be even slower than the current genksyms. For people who want genksyms (i.e. distros), don't they normally already enable DEBUG_INFO as well? The problems of genksyms are well known and a pain (I speak from experience), so replacing it with info based on DWARF would be great, I'll gladly trade off the DEBUG_INFO issue for stablilty! thanks, greg k-h
On Wed, Jun 19, 2024 at 2:18 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Wed, Jun 19, 2024 at 01:50:36AM +0900, Masahiro Yamada wrote: > > On Wed, Jun 19, 2024 at 1:44 AM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > > On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > > > > Hi folks, > > > > > > > > This series implements CONFIG_MODVERSIONS for Rust, an important > > > > feature for distributions like Android that want to ship Rust > > > > kernel modules, and depend on modversions to help ensure module ABI > > > > compatibility. > > > > > > > > There have been earlier proposals [1][2] that would allow Rust > > > > modules to coexist with modversions, but none that actually implement > > > > symbol versioning. Unlike C, Rust source code doesn't have sufficient > > > > information about the final ABI, as the compiler has considerable > > > > freedom in adjusting structure layout for improved performance [3], > > > > for example, which makes using a source code parser like genksyms > > > > a non-starter. Based on Matt's suggestion and previous feedback > > > > from maintainers, this series uses DWARF debugging information for > > > > computing versions. DWARF is an established and relatively stable > > > > format, which includes all the necessary ABI details, and adding a > > > > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > > > > reasonable trade-off. > > > > > > > > The first 12 patches of this series add a small tool for computing > > > > symbol versions from DWARF, called gendwarfksyms. When passed a list > > > > of exported symbols, the tool generates an expanded type string > > > > for each symbol, and computes symbol CRCs similarly to genksyms. > > > > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > > > > because of the existing support for C host tools that use elfutils > > > > (e.g., objtool). > > > > > > That's cool, can the C code be switched to also use this? That way we > > > only have one path/code for all of this? > > > > > > As the description says, it requires CONFIG_DEBUG_INFO. > > We can strip the debug info from the final vmlinux, but > > I guess the build speed will be even slower than the current genksyms. > > For people who want genksyms (i.e. distros), don't they normally already > enable DEBUG_INFO as well? The problems of genksyms are well known and > a pain (I speak from experience), so replacing it with info based on > DWARF would be great, I'll gladly trade off the DEBUG_INFO issue for > stablilty! > > thanks, > > greg k-h > I do not think gendwarfksyms is a drop-in replacement, because it relies on libelf and libdw, which will not work with LLVM bitcode when CONFIG_LTO_CLANG=y. His "Let's postpone this until final linking" stuff will come back? Then, vmlinux.o is processed to extract the CRC of all symbols? In my benchmark, this tool took 3.84 sec just for processing a single rust/core.o object. I'd love to see how long it will take to process vmlinux.o And this occurs even when a single source file is changed and vmlinux.o is re-linked. -- Best Regards Masahiro Yamada
On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > Hi folks, > > This series implements CONFIG_MODVERSIONS for Rust, an important > feature for distributions like Android that want to ship Rust > kernel modules, and depend on modversions to help ensure module ABI > compatibility. > > There have been earlier proposals [1][2] that would allow Rust > modules to coexist with modversions, but none that actually implement > symbol versioning. Unlike C, Rust source code doesn't have sufficient > information about the final ABI, as the compiler has considerable > freedom in adjusting structure layout for improved performance [3], > for example, which makes using a source code parser like genksyms > a non-starter. Based on Matt's suggestion and previous feedback > from maintainers, this series uses DWARF debugging information for > computing versions. DWARF is an established and relatively stable > format, which includes all the necessary ABI details, and adding a > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > reasonable trade-off. OK sure. > The first 12 patches of this series add a small tool for computing > symbol versions from DWARF, called gendwarfksyms. When passed a list > of exported symbols, the tool generates an expanded type string > for each symbol, and computes symbol CRCs similarly to genksyms. So this is too word centric Rust, let's think about this generically. We still ahve a symbol limitation even in the C world then, and this solution can solve that problem also for other reasons for *whatever* reason we devise to-come-up-with-in-the-future for augmenting symbols. Today Rust, tomorrow, who knows. > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > because of the existing support for C host tools that use elfutils > (e.g., objtool). I agree with Masahiro, that testing this with vmlinux would be eye opening to what challenges we really have ahead. So, to help with this let's try to re-think about this from another perspective. Yes, old userspace should not break, but you can add yet another option to let you opt-in to a new world order of how these crc are mapped to hashed repersentations of the symbols. This would allow us to: a) Ensure correctness for all users / tools, so that proper plumbing is really done. By considering all symbols you increase your scope of awareness of anything that could really break. b) Remove the "Rust" nature about this c) Rust modules just becomes a *user* of this approach It gets me wondering, given Kris is also working on allowing traces to map symbols to module names, how does this fit into that world [0]? As for a) the reason I'm thinking about having the ability to test a full real kernel and moules with this is, without that, how are you sure you have the full scope of the changes needed? [0] https://lkml.kernel.org/r/20240614171428.968174-3-kris.van.hees@oracle.com Luis
Hi Masahiro, On Wed, Jun 19, 2024 at 01:28:21AM +0900, Masahiro Yamada wrote: > I am surprised at someone who attempts to add another variant of genksyms. The options are rather limited if we want Rust modules that are compatible with modversions. We either come up with a way to version Rust symbols or we bypass version checks for them, which is basically what Matt's earlier patch set did: https://lore.kernel.org/rust-for-linux/20231118025748.2778044-1-mmaurer@google.com/ If there are better solutions, I would be happy to hear them. > I am also surprised at the tool being added under the tools/ directory. I picked this location because that's where objtool lives, and wasn't aware that this was frowned upon. I'm fine with moving the tool elsewhere too. Sami
On Wed, Jun 19, 2024 at 04:03:45AM +0900, Masahiro Yamada wrote: > On Wed, Jun 19, 2024 at 2:18 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Jun 19, 2024 at 01:50:36AM +0900, Masahiro Yamada wrote: > > > On Wed, Jun 19, 2024 at 1:44 AM Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > > That's cool, can the C code be switched to also use this? That way we > > > > only have one path/code for all of this? > > > > > > > > > As the description says, it requires CONFIG_DEBUG_INFO. > > > We can strip the debug info from the final vmlinux, but > > > I guess the build speed will be even slower than the current genksyms. > > > > For people who want genksyms (i.e. distros), don't they normally already > > enable DEBUG_INFO as well? The problems of genksyms are well known and > > a pain (I speak from experience), so replacing it with info based on > > DWARF would be great, I'll gladly trade off the DEBUG_INFO issue for > > stablilty! > > > > thanks, > > > > greg k-h > > > > > > I do not think gendwarfksyms is a drop-in replacement, > because it relies on libelf and libdw, which will not > work with LLVM bitcode when CONFIG_LTO_CLANG=y. > > His "Let's postpone this until final linking" stuff will > come back? > Then, vmlinux.o is processed to extract the CRC > of all symbols? I agree, this won't work with LTO unless we process vmlinux.o. > In my benchmark, this tool took 3.84 sec just for processing > a single rust/core.o object. To be fair, Rust currently exports all globals and core.o has 400 exported symbols as a result. During my brief testing, this tool is faster than genksyms for normal C code. > I'd love to see how long it will take to process vmlinux.o It's obviously going to be quite slow, my defconfig vmlinux.o has 14k exported symbols: Performance counter stats for './tools/gendwarfksyms/gendwarfksyms vmlinux.o': 371,527.67 msec task-clock:u # 1.000 CPUs utilized 0 context-switches:u # 0.000 /sec 0 cpu-migrations:u # 0.000 /sec 231,554 page-faults:u # 623.248 /sec <not supported> cycles:u <not supported> instructions:u <not supported> branches:u <not supported> branch-misses:u 371.686151684 seconds time elapsed 370.534637000 seconds user 0.987825000 seconds sys The tool is currently single-threaded, so if we really want to go this route, it could probably be made a bit faster. > And this occurs even when a single source file is changed > and vmlinux.o is re-linked. I suppose anyone using LTO already knows it won't be a quick rebuild though. Sami
Hi Luis, On Tue, Jun 18, 2024 at 12:42:51PM -0700, Luis Chamberlain wrote: > On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > > The first 12 patches of this series add a small tool for computing > > symbol versions from DWARF, called gendwarfksyms. When passed a list > > of exported symbols, the tool generates an expanded type string > > for each symbol, and computes symbol CRCs similarly to genksyms. > > So this is too word centric Rust, let's think about this generically. > We still ahve a symbol limitation even in the C world then, and this > solution can solve that problem also for other reasons for *whatever* > reason we devise to-come-up-with-in-the-future for augmenting symbols. > Today Rust, tomorrow, who knows. If you're referring to the symbol hashing in the __versions table, that's not specific to Rust. Rust just happens to be the only source of long symbol names right now. > > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > > because of the existing support for C host tools that use elfutils > > (e.g., objtool). > > I agree with Masahiro, that testing this with vmlinux would be eye > opening to what challenges we really have ahead. So, to help with this > let's try to re-think about this from another perspective. > > Yes, old userspace should not break, but you can add yet another option > to let you opt-in to a new world order of how these crc are mapped to > hashed repersentations of the symbols. This would allow us to: We did run into an issue with depmod in our testing, where it needs to be taught about hashed names to avoid 'unknown symbol' warnings. I'm not sure if this is acceptable, so I would like to hear feedback about the viability of the hashing scheme in general. If old userspace can't have any regressions because of long symbol names, I suspect we'll have to go back to omitting long symbols from struct modversion_info and adding a v2 of the __versions table with no symbol name length limitations. Happy to hear your thoughts. > a) Ensure correctness for all users / tools, so that proper plumbing is > really done. By considering all symbols you increase your scope of > awareness of anything that could really break. > > b) Remove the "Rust" nature about this > > c) Rust modules just becomes a *user* of this approach I believe the only Rust nature here is the last patch that enables gendwarfksyms only for Rust. Otherwise, there shouldn't be anything strictly Rust specific. > It gets me wondering, given Kris is also working on allowing traces to > map symbols to module names, how does this fit into that world [0]? AFAIK long symbol names are only a problem for struct modversion_info, which uses a relatively short name buffer, so I'm hoping other efforts won't be affected. > As for a) the reason I'm thinking about having the ability to test a > full real kernel and moules with this is, without that, how are you sure > you have the full scope of the changes needed? Sure, I can look into hooking this up for the entire kernel and seeing what breaks, in addition the issues Masahiro pointed out, of course. Sami
On Tue, Jun 18, 2024 at 02:19:47PM -0700, Sami Tolvanen wrote: > Hi Luis, > > On Tue, Jun 18, 2024 at 12:42:51PM -0700, Luis Chamberlain wrote: > > a) Ensure correctness for all users / tools, so that proper plumbing is > > really done. By considering all symbols you increase your scope of > > awareness of anything that could really break. > > > > b) Remove the "Rust" nature about this > > > > c) Rust modules just becomes a *user* of this approach > > I believe the only Rust nature here is the last patch that enables > gendwarfksyms only for Rust. Otherwise, there shouldn't be anything > strictly Rust specific. Right now the check for length is generic, and assumes a hash may exist without considering that check is moot for non-rust modules. So the inverse is true, but doesn't provide a solution or proper architecture for it. > > It gets me wondering, given Kris is also working on allowing traces to > > map symbols to module names, how does this fit into that world [0]? > > AFAIK long symbol names are only a problem for struct modversion_info, > which uses a relatively short name buffer, so I'm hoping other efforts > won't be affected. We'll see! > > As for a) the reason I'm thinking about having the ability to test a > > full real kernel and moules with this is, without that, how are you sure > > you have the full scope of the changes needed? > > Sure, I can look into hooking this up for the entire kernel and seeing > what breaks, in addition the issues Masahiro pointed out, of course. :) should be fun! I think once its revised as a "generic" strategy and not a Rust one, the proper architecture will be considered. Right now, it just looks like a cheap band aid for Rust. Luis