[Linux-kernel-mentees] [PATCH] events: Annotate parent_ctx with __rcu

Amol Grover frextrite at gmail.com
Mon Feb 10 12:59:48 UTC 2020


On Mon, Feb 10, 2020 at 10:36:24AM +0100, Peter Zijlstra wrote:
> On Sat, Feb 08, 2020 at 08:16:49PM +0530, Amol Grover wrote:
> > parent_ctx is used under RCU context in kernel/events/core.c,
> > tell sparse about it aswell.
> > 
> > Fixes the following instances of sparse error:
> > kernel/events/core.c:3221:18: error: incompatible types in comparison
> > kernel/events/core.c:3222:23: error: incompatible types in comparison
> > 
> > This introduces the following two sparse errors:
> > kernel/events/core.c:3117:18: error: incompatible types in comparison
> > kernel/events/core.c:3121:30: error: incompatible types in comparison
> > 
> > Hence, use rcu_dereference() to access parent_ctx and silence
> > the newly introduced errors.
> > 
> > Signed-off-by: Amol Grover <frextrite at gmail.com>
> > ---
> >  include/linux/perf_event.h |  2 +-
> >  kernel/events/core.c       | 11 ++++++++---
> >  2 files changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> > index 6d4c22aee384..e18a7bdc6f98 100644
> > --- a/include/linux/perf_event.h
> > +++ b/include/linux/perf_event.h
> > @@ -810,7 +810,7 @@ struct perf_event_context {
> >  	 * These fields let us detect when two contexts have both
> >  	 * been cloned (inherited) from a common ancestor.
> >  	 */
> > -	struct perf_event_context	*parent_ctx;
> > +	struct perf_event_context __rcu	*parent_ctx;
> >  	u64				parent_gen;
> >  	u64				generation;
> >  	int				pin_count;
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 2173c23c25b4..2a8c5670b254 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -3106,26 +3106,31 @@ static void ctx_sched_out(struct perf_event_context *ctx,
> >  static int context_equiv(struct perf_event_context *ctx1,
> >  			 struct perf_event_context *ctx2)
> >  {
> > +	struct perf_event_context *parent_ctx1, *parent_ctx2;
> > +
> >  	lockdep_assert_held(&ctx1->lock);
> >  	lockdep_assert_held(&ctx2->lock);
> >  
> > +	parent_ctx1 = rcu_dereference(ctx1->parent_ctx);
> > +	parent_ctx2 = rcu_dereference(ctx2->parent_ctx);
> 
> Bah.
> 
> Why are you  fixing all this sparse crap and making the code worse?

Hi Peter,

Sparse is quite noisy and we need to eliminate false-positives, right?
__rcu will tell the developer, this pointer could change and he needs to
take the required steps to make sure the code doesn't break.

Thanks
Amol


More information about the Linux-kernel-mentees mailing list