Re: [PATCH v4 04/17] context_tracking: Add ct_state and CT_WARN_ON

[Ingo Molnar <mingo@kernel.org>]

* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Jun 29, 2015 at 12:33:36PM -0700, Andy Lutomirski wrote:
> > This will let us sprinkle sanity checks around the kernel without
> > making too much of a mess.
> > 
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >  include/linux/context_tracking.h       | 15 +++++++++++++++
> >  include/linux/context_tracking_state.h |  1 +
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> > index b96bd299966f..008fc67d0d96 100644
> > --- a/include/linux/context_tracking.h
> > +++ b/include/linux/context_tracking.h
> > @@ -49,13 +49,28 @@ static inline void exception_exit(enum ctx_state prev_ctx)
> >  	}
> >  }
> >  
> > +
> > +/**
> > + * ct_state() - return the current context tracking state if known
> > + *
> > + * Returns the current cpu's context tracking state if context tracking
> 
> 			 CPU's
> 
> > + * is enabled.  If context tracking is disabled, returns
> > + * CONTEXT_DISABLED.  This should be used primarily for debugging.
> > + */
> > +static inline enum ctx_state ct_state(void)
> > +{
> > +	return context_tracking_is_enabled() ?
> > +		this_cpu_read(context_tracking.state) : CONTEXT_DISABLED;
> > +}
> >  #else
> >  static inline void user_enter(void) { }
> >  static inline void user_exit(void) { }
> >  static inline enum ctx_state exception_enter(void) { return 0; }
> >  static inline void exception_exit(enum ctx_state prev_ctx) { }
> > +static inline enum ctx_state ct_state(void) { return CONTEXT_DISABLED; }
> >  #endif /* !CONFIG_CONTEXT_TRACKING */
> >  
> > +#define CT_WARN_ON(cond) WARN_ON(context_tracking_is_enabled() && (cond))
> 
> Btw, that "CT_" prefix makes this look like it is the software
> controlling this thing:
> 
> https://upload.wikimedia.org/wikipedia/commons/thumb/2/27/UPMCEast_CTscan.jpg/1280px-UPMCEast_CTscan.jpg
> 
> :-)
> 
> Other kernel code uses "cxt" or "ctxt" abbreviations. Maybe
> 
> 	CTXT_WARN_ON(cond)

So this really looks too much like 'TXT' - which is very far from what it tries to 
express.

So we should either use a CTX_TRACK_ prefix that is unambiguous, or live with CT_ 
and its ambiguity. (There's another ambiguity it has: 'ct' stands for 'connection 
tracking' in the networking code.)

Thanks,

	Ingo