Re: [PATCH v3 5/5] evtchn: don't call Xen consumer callback with per-channel lock held

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 23/12/2020 14:56, Jan Beulich wrote:
> On 23.12.2020 15:44, Julien Grall wrote:
> > On 23/12/2020 13:41, Jan Beulich wrote:
> > > On 23.12.2020 14:33, Julien Grall wrote:
> > > > On 23/12/2020 13:12, Jan Beulich wrote:
> > > > >   From the input by both of you I still can't
> > > > > conclude whether this patch should remain as is in v4, or revert
> > > > > back to its v2 version. Please can we get this settled so I can get
> > > > > v4 out?
> > > > I haven't had time to investigate the rest of the VM event code to find
> > > > other cases where this may happen. I still think there is a bigger
> > > > problem in the VM event code, but the maintainer disagrees here.
> > > >
> > > > At which point, I see limited reason to try to paper over in the common
> > > > code. So I would rather ack/merge v2 rather than v3.
> > > Since I expect Tamas and/or the Bitdefender folks to be of the
> > > opposite opinion, there's still no way out, at least if "rather
> > > ack" implies a nak for v3.
> > The only way out here is for someone to justify why this patch is
> > sufficient for the VM event race.
> I think this is too much you demand.
I guess you didn't notice that I did most of the job by providing an 
analysis in my e-mail... I don't think it is too much demanding to read 
the analysis and say whether I am correct or not.
Do you really prefer to add code would get rot because unused?

> Moving in the right direction
> without arriving at the final destination is still a worthwhile
> thing to do imo.
>
> > > Personally, if this expectation of
> > > mine is correct, I'd prefer to keep the accounting but make it
> > > optional (as suggested in a post-commit-message remark).
> > > That'll eliminate the overhead you appear to be concerned of,
> > > but of course it'll further complicate the logic (albeit just
> > > slightly).
> > I am more concerned about adding over complex code that would just
> > papering over a bigger problem. I also can't see use of it outside of
> > the VM event discussion.
> I think it is a generally appropriate thing to do to wait for
> callbacks to complete before tearing down their origin control
> structure. There may be cases where code structure makes this
> unnecessary, but I don't think this can be an expectation to
> all the users of the functionality. Hence my suggestion to
> possibly make this optional (driven directly or indirectly by
> the user of the registration function).
As you tend to say, we should not add code unless there is a user. So 
far the only possible user is dubbious. If you find another user, then 
we can discuss whether this patch makes sense.
Cheers,

--
Julien Grall