On Mon, 17 Dec 2018, Joe Lawrence wrote: > On 12/17/2018 07:03 AM, Miroslav Benes wrote: > > Hi, > > > > I'm sorry for being late to the party. > > > > On Sun, 16 Dec 2018, Nicholas Mc Guire wrote: > > > >> Sparse reported warnings about non-static symbols. For the variables > >> a simple static attribute is fine - for those symbols referenced by > >> livepatch via klp_func the symbol-names must be unmodified in the > >> symbol table - to resolve this the __noclone attribute is used > >> for the shared statically declared functions. > >> > >> Signed-off-by: Nicholas Mc Guire <hof...@osadl.org> > >> Suggested-by: Joe Lawrence <joe.lawre...@redhat.com> > >> Link: https://lkml.org/lkml/2018/12/13/827 > > > > A nit, but I'd reorder the tags. Link, Suggested-by:, Signed-off-by:. Also > > it would be great if you used https://lkml.kernel.org/r/${Msg-ID} > > redirection. > > > >> --- > >> > >> V2: not all static functions shared need to carry the __noclone > >> attribute only those that need to be resolved at runtime by > >> livepatch - so drop the unnecessary __noclone attributes as > >> well as the Note on __noclone as suggested by Joe Lawrence > >> <joe.lawre...@redhat.com> - thanks ! > > > > I talked to Martin Jambor (GCC) and he suggested __attribute__((used)). It > > should be better than __noclone, which was reportedly implemented only for > > testing purposes (which is why it does not imply noinline, although > > inlining internally uses cloning). Newer gcc also has "noipa" attribute, > > but "used" would definitely be safe. > > > > Sorry for not responding earlier. > > > > Hi Miroslav, > > Thanks for following up on this. "noipa" would have been nice to use, > but as you say, is a newer gcc attribute. > > Regarding "used" vs. "noclone", can we assume that "used" implies that > the function name remains unchanged?
I am not sure. I'd argue that it does imply that, but it could just be a consequence without any guarantees. My understanding is that gcc cannot assume about a symbol and its references. So it should be preserved as is. > The gcc online doc [1] speaks about ensuring that "code must be > emitted". This reads like it solves our > static-function-not-directly-referenced problem, but isn't explicit > about naming. Correct. > used > > This attribute, attached to a function, means that code must be > emitted for the function even if it appears that the function is not > referenced. This is useful, for example, when the function is > referenced only in inline assembly. > > Perhaps it's equivalent to a "I want to keep this function and leave > it's symbols alone" attribute. FWIW, I modified Nicholas' change on my > box to use "used" and it worked as Martin advertised, so it would get my > Ack. > > I'm just being picky about its documentation and how we should note its > usage in the v3 patch. Think that s/__noclone/used/g of the v2 commit > message would be sufficient? We could rephrase it. After all it is not only about symbol names in the symbol table. The traceable/patchable code has to be present... "Sparse reported warnings about non-static symbols. For the variables a simple static attribute is fine - for the functions referenced by livepatch via klp_func the symbol-names must be unmodified in the symbol table and the patchable code has to be emitted. Attach __used attribute to the shared statically declared functions." ? Miroslav
