On Tue, Jan 24, 2023 at 07:50:31AM -0700, Jonathan Corbet wrote: > David Vernet <void@xxxxxxxxxxxxx> writes: > > > I was perhaps a bit naive to think we could just throw a __bpf_kfunc > > macro onto the function signatures and call it a day :-) I think it's > > probably best to table this for now, and either I or someone else can > > come back to it when we have bandwidth to solve the problem more > > appropriately. > > Now I feel bad ... I was just tossing out a thought, not wanting to > bikeshed this work into oblivion. If what you have solves a real No apologies necessary. I don't think this qualifies as bikeshedding. IMO folks are raising legitimate UX concerns, which is important and worth getting right. > problem and is the best that can be done now, perhaps it should just go > in and a "more appropriate" solution can be adopted later, should > somebody manage to come up with it? That would be my preference, but I also understand folks' sentiment of wanting to keep out what they feel like is odd syntax, as Christoph said in [0], and Daniel alluded to earlier in this thread. [0]: https://lore.kernel.org/all/Y8+FeH7rz8jDTubt@xxxxxxxxxxxxx/ I tested on an LTO build and wrapper kfuncs (with external linkage) were not being stripped despite not being called from anywhere else in the kernel, so for now I _think_ it's safe to call this patch set more of a cleanup / future-proofing than solving an immediate and pressing problem (as long as anyone adding kfuncs carefully follows the directions in [1]). In other words, I think we have some time to do this the right way without paying too much of a cost later. If we set up the UX correctly, just adding an EXPORT_SYMBOL_KFUNC call (or something to that effect, including just using BTF_ID_FLAGS) should be minimal effort even if there are a lot more kfuncs by then. [1]: https://docs.kernel.org/bpf/kfuncs.html If it turns out that we start to observe problems in LTO builds without specifying __used and/or noinline, or if folks are repeatedly making mistakes when adding kfuncs (by e.g. not giving wrapper kfuncs external linkage) then I think it would be a stronger case to get this in now and fix it up later. Thanks, David
