Re: [kvm-unit-tests PATCH v2 2/2] s390x: firq: floating interrupt test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/12/2021 19.23, David Hildenbrand wrote:

+	if (smp_query_num_cpus() < 3) {
+		report_skip("need at least 3 CPUs for this test");
+		goto out;
+	}
+
+	if (stap()) {
+		report_skip("need to start on CPU #0");
+		goto out;
+	}

I think I'd rather turn this into an assert() instead ... no strong opinion
about it, though.

I agree, including the part about no strong opinions (which is why I
did not comment on it before)

Would it be the case on any system we might end up running, even under
LPAR ... and whoever could run these tests ?

Well, ok, since it likely doesn't happen in real life anyway, simply keep the report_skip().



+
+	/*
+	 * We want CPU #2 to be stopped. This should be the case at this
+	 * point, however, we want to sense if it even exists as well.
+	 */
+	ret = smp_cpu_stop(2);
+	if (ret) {
+		report_skip("CPU #2 not found");

Since you already queried for the availablity of at least 3 CPUs above, I
think you could turn this into a report_fail() instead?

either that or an assert, but again, no strong opinions


Just because there are >= 3 CPUs doesn't imply that CPU #2 is around.

Ok, fair point. But if #2 is not around, it means that the test has been run in the wrong way by the user... I wonder what's better in that case - to skip this test or to go out with a bang. Skipping the test has the advantage of looking a little bit more "polite", but it has the disadvantage that it might get lost in automation, e.g. if somebody enabled the test in their CI, but did something wrong in the settings, they might not notice that the test is not run at all...

What we could remove is the "if (smp_query_num_cpus() < 3) {" check, though!

Yes, that seems to be redundant, indeed.

 Thomas




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux