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

Re: [Xen-devel] [PATCH v3 4/6] Pause/Unpause the domain before/after assigning PI hooks



>>> On 20.09.16 at 01:12, <dario.faggioli@xxxxxxxxxx> wrote:
> On Sun, 2016-09-18 at 08:37 +0000, Wu, Feng wrote:
>> > From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
>> However, I found this is not true in some scenarios, such as,
>> vmx_pi_switch_to() hasn't been installed or executed before
>> vmx_vcpu_block() gets called, in which case, we may hit the ASSERT
>> in it. In previous version, I suggested we remove the ASSERT in
>> vmx_vcpu_block() and set NDST explicitly in it. But seems maintainer
>> doesn't like this idea. 
>>
> And this is not helping either. An ASSERT() being hit means something
> wrong happened. Whether or not to remove (or change) an ASSERT() is not
> a matter of "like".
> 
> If we hit the ASSERT() but nothing is wrong with the code, then it is
> the ASSERT() itself that is wrong (buggy), and we must remove it, like
> it or not.
> 
> OTOH, if we hit a non buggy ASSERT(), then it means that the ASSERT()
> has done its job in finding something wrong in the code, and we should
> leave it alone and fix the problem.
> 
> How's possible for the solution here to be "either remove the ASSERT()
> _OR_ change the code"? That really makes few sense to me... :-O

I disagree: Whether fixing/removing an ASSERT() that triggered or
adjusting other code to make the ASSERT() not trigger can indeed
both be an option, and may to some degree be a matter of taste.
And that's where I think Feng and I disagree - I'd prefer the ASSERT()
to stay, and code elsewhere to make sure it won't trigger, while he
would prefer to get rid of the ASSERT() (aiui on the basis that the
code following the ASSERT() establishes the intended state - Feng,
please correct me if I'm wrong).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.