Re: kmemleak handling of kfree_rcu

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Sep 06, 2023 at 12:11:12PM -0700, Paul E. McKenney wrote:
> On Wed, Sep 06, 2023 at 06:15:49PM +0100, Catalin Marinas wrote:
> > On Wed, Sep 06, 2023 at 02:35:29PM +0000, Joel Fernandes wrote:
> > > On Tue, Sep 05, 2023 at 03:41:32PM +0100, Catalin Marinas wrote:
> > > > On Tue, Sep 05, 2023 at 11:17:25AM +0000, Joel Fernandes wrote:
> > > > > The correct fix then should probably be to mark the object as
> > > > > kmemleak_not_leak() until a grace period elapses. This will cause the object
> > > > > to not be reported but still be scanned until eventually the lower layers
> > > > > will remove the object from kmemleak-tracking after the grace period. Per the
> > > > > docs also, that API is used to prevent false-positives.
> > > > 
> > > > This should work as well but I'd use kmemleak_ignore() instead of
> > > > kmemleak_not_leak(). The former, apart from masking the false positive,
> > > > also tells kmemleak not to scan the object. After a kvfree_rcu(), the
> > > > object shouldn't have any valid references to other objects, so not
> > > > worth scanning.
> > > 
> > > Yes I am also OK with that, however to me I consider the object as alive as
> > > long as the grace period does not end. But I agree with you and it may not be
> > > worth tracking them or scanning them.
> > 
> > I guess from an RCU perspective, the object is still alive. From the
> > kvfree_rcu() caller perspective though, it can disappear at any point
> > after the grace period, so it shouldn't rely on its content being valid
> > and referencing other objects (other than transiently e.g. in RCU list
> > traversal).
> > 
> > It probably only matters if we have some very long grace periods (I'm
> > not up to date with the recent RCU developments). In such cases, the
> > object still being scanned could introduce false negatives. That's my
> > reasoning for suggesting kmemleak_ignore() rather than
> > kmemleak_not_leak().
> 
> Very long RCU readers still result in very long RCU grace periods.  And,
> after some tens of seconds, RCU CPU stall warnings.  So don't let your
> RCU readers run for that long.  But you knew that already.  ;-)

That's still ok. I was more thinking of deferred freeing well past the
RCU readers completing.

> > @@ -3388,6 +3389,14 @@ void kvfree_call_rcu(struct rcu_head *head, void *ptr)
> >  		success = true;
> >  	}
> >  
> > +	/*
> > +	 * The kvfree_rcu() caller considers the pointer freed at this point
> > +	 * and likely removes any references to it. Since the the actual slab
> > +	 * freeing (and kmemleak_free()) is deferred, tell kmemleak to ignore
> > +	 * this object (no scanning or false positives reporting).
> > +	 */
> > +	kmemleak_ignore(ptr);
> 
> Do we want to un-ignore it at the end of the grace period?  Or will that
> happen automatically when it is passed to kfree()?  (My guess is that
> the answer to both questions is "yes", but I figured that I should ask.)

No need to un-ignore. This function only tells kmemleak it's not a
leak and doesn't have any interesting data to scan. Kmemleak still keeps
track of it until properly freed.

-- 
Catalin