[Pdbg] [PATCH 7/7] pdbg/htm: Enforce powersave=off

[Cyril Bur]

On Fri, 2018-03-16 at 13:53 +1100, Michael Neuling wrote:
> Why not just use the sysfs interface to turn off the cpuidle driver?
> 

I can add that as another hint to the user. It isn't pdbgs place to
change the state of the system but providing the most user friendly
hints is!

> Mikey
> 
> 
> On Wed, 2018-03-07 at 16:49 +1100, Cyril Bur wrote:
> > Core HTM has a nasty habit of not working if the stars haven't aligned
> > with the exact polar axis of Mars. To complicate matters if Zeus got up
> > on the incorrect side of the bed Core HTM will likely reflect his mood.
> > 
> > Core HTM requires that no threads be in powersave. It isn't clear if
> > this restriction only applies to threads on the core being traced on or
> > if this is a global thing. In order to be as safe as possible, pdbg
> > will enforce that powersave be disabled globally.
> > 
> > Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
> > ---
> > Initially I thought we would also have to mandate `ppc64_cpu --smt=off`
> > because other tools do it. Some thinking later it doesn't make sense to
> > enforce that, really the only thing we have to do with respect to Linux
> > is powersave=off, having a sleeping thread while core trace is
> > guaranteed to do bad things. I have a feeling that other tools got a
> > bit confused between powersave and smt (at least the Linux view of both).
> > 
> > It works on a Palmetto with `ppc64_cpu --smt=8`. I am waiting on
> > confirmation that the resulting trace is sane but I don't have any
> > reason to believe it isn't
> > ---
> >  src/htm.c | 37 ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/htm.c b/src/htm.c
> > index 1032a5c..4917d1a 100644
> > --- a/src/htm.c
> > +++ b/src/htm.c
> > @@ -259,6 +259,7 @@ static void print_usage(enum htm_type type)
> >  
> >  int run_htm(int optind, int argc, char *argv[])
> >  {
> > +	struct pdbg_target *target;
> >  	enum htm_type type;
> >  	int i, rc = 0;
> >  
> > @@ -278,9 +279,43 @@ int run_htm(int optind, int argc, char *argv[])
> >  		return 0;
> >  	}
> >  
> > -	if (type == HTM_CORE)
> > +	if (type == HTM_CORE) {
> >  		fprintf(stderr, "Warning: Core HTM is currently
> > experimental\n");
> >  
> > +		/*
> > +		 * Check that powersave is off.
> > +		 *
> > +		 * This is as easy as checking that every single
> > +		 * thread is "ACTIVE" and hasn't gone into any sleep
> > +		 * state.
> > +		 */
> > +		pdbg_for_each_class_target("thread", target) {
> > +			if (pdbg_target_status(target) ==
> > PDBG_TARGET_NONEXISTANT)
> > +				continue;
> > +
> > +			if ((thread_status(target) & THREAD_STATUS_ACTIVE) !=
> > THREAD_STATUS_ACTIVE) {
> > +				fprintf(stderr, "It appears powersave is on
> > 0x%"  PRIx64 "%p\n", thread_status(target), target);
> > +				fprintf(stderr, "core HTM needs to run with
> > powersave off\n");
> > +				fprintf(stderr, "Hint: put powersave=off on
> > the kernel commandline\n");
> > +				return 0;
> > +			}
> > +		}
> > +
> > +		i = 0;
> > +		pdbg_for_each_class_target("core", target)
> > +			if (!target_is_disabled(target))
> > +				i++;
> > +		if (i == 0)
> > +			fprintf(stderr, "You haven't selected any HTM
> > Cores\n");
> > +		if (i > 1) {
> > +			fprintf(stderr, "It doesn't make sense to core trace
> > on"
> > +				" multiple cores at once. %d\n", i);
> > +			fprintf(stderr, "What you probably want is -p 0 -c
> > x\n");
> > +		}
> > +		if (i != 1)
> > +			return 0;
> > +	}
> > +
> >  	optind++;
> >  	for (i = 0; i < ARRAY_SIZE(actions); i++) {
> >  		if (strcmp(argv[optind], actions[i].name) == 0) {
> > -- 
> > 2.16.2
> >