Re: [PATCH 2/3] livepatch/rcu: Warn when system consistency is broken in RCU code

[Miroslav Benes <mbenes@suse.cz>]

Being somewhat late to the party I missed all the fun...

On Wed, 10 May 2017, Josh Poimboeuf wrote:

> On Wed, May 10, 2017 at 06:04:23PM +0200, Petr Mladek wrote:
> 
> > IMHO, the point is that RCU must be aware when we call
> > rcu_read_lock()/unlock().
> > 
> > My understanding is that rcu_irq_enter() tries to make RCU watching
> > when it was not. Then rcu_is_watching() reports if we are on
> > the safe side.
> > 
> > But it is possible that I miss something. One question is if
> > rcu_irq_enter()/exit() calls can be nested.
> > 
> > I still need to think about it.
> 
> The code looks ok to me now, except for a few minor issues:
> 
> - The warning message should be more specific.
> 
> - The documentation should probably mention the name of the specific RCU
>   function which shouldn't be patched.
> 
> - The documentation might also mention that the warning could also be
>   triggered in early NMI or exception code, e.g. if there are any calls
>   to functions with fentry calls which have been patched.
> 
> - The code comment should probably refer to the documentation, otherwise
>   nobody will ever read it ;-)

Agreed.

Petr, could you also improve the changelog a bit? Certain parts confuse 
me and I think that your changes to the documentation describe it better.

I mean these two paragraphs:

"Unfortunately, the ftrace handler might be called when the problematic
patch has already been removed from ops->func stack. In this case,
it is not able to read the immediate flag. It makes the check
unreliable. We rather avoid it and report the problem even when
the system stability is not affected.

It would be possible to add some more complex logic to avoid
warnings when RCU infrastructure is modified using immediate
patches. But let's keep it simple until real life experience
forces us to do the opposite."

I'm still not sure if we know for 100 percent what we're doing :)

Miroslav