Re: [LTP] [PATCH v3] thermal: add new test group

[Petr Vorel <pvorel@suse.cz>]

Hi Piotr,

> > > Then it
> > > + * decreases the threshold for sending a thermal interrupt to just
> > > above
> > > + * the current temperature and runs a workload on the CPU.

> > First, why test needs to run for 30 sec and then sleep for 10 sec?
Maybe the most important of my questions / points.

> Here the point is to use a decreasing timeout. The test starts with 10s
> cooldown to make sure that even pre-production CPU's, which might have
> their thermal protections disabled, cool down properly. Once sleep time
> reaches 0, the conclusion is that either there was not enough workload
> or somehow interrupts are not triggered after all.

Why 30 sec and then sleep for 10 sec? Is it really needed to do it this way?
Aren't these times depending on the tested machine? Some of them will fail due
time not running enough, other will waste time (if they get interrupt e.g. in 10
sec). The usual approach would be to have the timeout safe enough for any type
of hardware but proactively check the temperature and stop testing once it's
done.

...
> > > +			int temp;
> > > +
> > > +			snprintf(path, PATH_LEN,
> > > "/sys/class/thermal/thermal_zone%d/", i);
> > > +			strncpy(temp_path, path, PATH_LEN);
> > > +			strncat(temp_path, "temp", 4);
> > > +			tst_res(TDEBUG, "Testing %s", temp_path);
> > nit: I'd put this as TINFO to get at least some debug info without -
> > D.

> > > +			SAFE_FILE_SCANF(temp_path, "%d", &temp);
> > All SAFE_*() macros quit testing, therefore the following check is
> > not needed.
> It's necessary because if the temperature is below 0, there's most
> likely some kernel or sensor issue.

I'm sorry, I was wrong here. I mixed up vfscanf() return value from LTP
function, but you check the scanned value.

Kind regards,
Petr

> > > +			if (temp < 0) {
> > > +				tst_brk(TBROK, "Unexpected zone
> > > temperature value %d", temp);
> > > +				status = 0;
> > > +			}
> > > +			tst_res(TDEBUG, "Current temperature for
> > > %s: %d", path, temp);

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp