On 17/05/2024 22:09, Eduard Zingerman wrote: > On Fri, 2024-05-17 at 11:22 +0100, Alan Maguire wrote: > [...] > >> Changes since v3[3]: >> >> - distill now checks for duplicate-named struct/unions and records >> them as a sized struct/union to help identify which of the >> multiple base BTF structs/unions it refers to (Eduard, patch 1) > > Hi Alan, > > Sorry, a little bit more on this topic. > - In patch #1 two kinds of structs get BTF_KIND_STRUCT declaration > with size: those embedded and those with ambiguous name. > - In patch #7 btf_relocate_map_distilled_base() unconditionally > requires size field for BTF_KIND_STRUCT to match. > > This might hinder portability in the following scenario: > - base type is referred to only by pointer; > - base type has ambiguous name (in the old kernel used for base BTF > generation); > - base type size is different in the new kernel when module is loaded. > > There is also a scenario when type name is not ambiguous in the old > kernel, but becomes ambiguous in the new kernel. > > So, what I had in mind when commented in v3 was: > - if distilled base has FWD for a structure and an ambiguous name, > fail to relocate; > - if distilled base has STRUCT for a structure, find a unique pair > name/size or fail to relocate. > > This covers scenario #1 but ignores scenario #2 and requires minimal > changes for v3 design. > > An alternative would be to e.g. keep STRUCT with size for all > structures in the base BTF and to compute "embedded" flag during > relocation: > - if distilled base STRUCT is embedded, search for a unique pair > name/size or fail to relocate; > - if distilled base STRUCT is not embedded, search for a uniquely > named struct, if that fails search for a unique pair name/size, > or fail to relocate. > Hi Eduard, IIRC I think Andrii suggested something like the above; then the idea was to use a 0 STRUCT size for cases where we didn't care about matching size rather than using a mix of FWDs and STRUCTs. As you say though, conditions can be different in the target base such that we might have wished we had recorded additional size information, and we can only really know that at relocation time. That being the case, I think the most robust approach is as you suggest to always record STRUCT/UNION size at distillation time and then only require size matches for the embedded and duplicate name-kind cases. This requires moving the embeddedness/dup logic from distillation to relocation. > If we consider above to much of a hassle, I think v3 design + size > check for STRUCT is better because it is a bit simpler. > > Wdyt? > The main goal from my side is to try and ensure we maximize the opportunities to reconcile split and base BTF where possible. The scheme you suggest above - always recording size but selectively using it in matching - seems like the best way to achieve that. Thanks! Alan > [...]
