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

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Tuesday, September 20, 2016 3:32 PM
> To: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Wu, Feng
> <feng.wu@xxxxxxxxx>; Tian, Kevin <kevin.tian@xxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxx
> Subject: Re: [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).

Yes, you are correct. Actually both solutions can resolve the issue.
And after some discussion, we are agree on fixing thing in the other
place to make we won't tirgger the ASSERT().

Thanks,
Feng

> 
> Jan


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