All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Claudio Imbrenda <imbrenda@linux.ibm.com>,
	Thomas Huth <thuth@redhat.com>
Cc: kvm@vger.kernel.org, Janosch Frank <frankja@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Sebastian Mitterle <smitterl@redhat.com>,
	Halil Pasic <pasic@linux.ibm.com>,
	linux-s390@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH v2 2/2] s390x: firq: floating interrupt test
Date: Fri, 3 Dec 2021 19:23:48 +0100	[thread overview]
Message-ID: <fa95d6e6-27be-7abf-7b1e-bb6bb9d62214@redhat.com> (raw)
In-Reply-To: <20211203121819.145696b0@p-imbrenda>


>>> +	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 ?

> 
>>
>>> +
>>> +	/*
>>> +	 * 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.

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

Thanks!

-- 
Thanks,

David / dhildenb


  parent reply	other threads:[~2021-12-03 18:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-02 12:35 [kvm-unit-tests PATCH v2 0/2] s390x: firq: floating interrupt test David Hildenbrand
2021-12-02 12:35 ` [kvm-unit-tests PATCH v2 1/2] s390x: make smp_cpu_setup() return 0 on success David Hildenbrand
2021-12-02 12:35 ` [kvm-unit-tests PATCH v2 2/2] s390x: firq: floating interrupt test David Hildenbrand
2021-12-02 12:45   ` Claudio Imbrenda
2021-12-03 10:55   ` Thomas Huth
2021-12-03 11:18     ` Claudio Imbrenda
2021-12-03 11:22       ` Thomas Huth
2021-12-03 18:23       ` David Hildenbrand [this message]
2021-12-06  7:12         ` Thomas Huth
2021-12-06  8:15           ` David Hildenbrand
2021-12-06 11:09             ` Claudio Imbrenda
2021-12-06 13:35 ` [kvm-unit-tests PATCH v2 0/2] " Claudio Imbrenda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fa95d6e6-27be-7abf-7b1e-bb6bb9d62214@redhat.com \
    --to=david@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pasic@linux.ibm.com \
    --cc=smitterl@redhat.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.