Re: [PATCH -tip v3 1/2] kcov: Make runtime functions noinstr-compatible

[Marco Elver <elver@google.com>]

On Wed, Jun 17, 2020 at 06:36PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 17, 2020 at 05:55:17PM +0200, Peter Zijlstra wrote:
> > On Wed, Jun 17, 2020 at 05:19:59PM +0200, Marco Elver wrote:
> > 
> > > > Does GCC (8, as per the new KASAN thing) have that
> > > > __builtin_memcpy_inline() ?
> > > 
> > > No, sadly it doesn't. Only Clang 11. :-/
> > > 
> > > But using a call to __memcpy() somehow breaks with Clang+KCSAN. Yet,
> > > it's not the memcpy that BUGs, but once again check_preemption_disabled
> > > (which is noinstr!). Just adding calls anywhere here seems to results in
> > > unpredictable behaviour. Are we running out of stack space?
> > 
> > Very likely, bad_iret is running on that entry_stack you found, and as
> > you found, it is puny.
> > 
> > Andy wanted to make it a full page a while ago, so I suppose the
> > question is do we do that now?
> 
> Andy suggested doing the full page; untested patches here:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/entry

Yeah, that works, thanks! I think the stack increase alone fixes any
kind of crash due to the reproducer.

Also, my guess is this is not a hot function, right? One caveat to keep
in mind is that because it's not 'memcpy', the compiler will never
inline these memcpys (unlike before). Whether or not that actually makes
things faster or slower is anyone's guess though.

Thanks,
-- Marco