Re: [Xen-devel] [RFC 1/6] xen/arm: Re-enable interrupt later in the trap path

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrii,

On 7/30/19 6:35 PM, Andrii Anisov wrote:
> On 26.07.19 13:48, Julien Grall wrote:
> > This is the 3rd time you send this patch... and still no proper 
> > explanation why this is done nor the impact on keeping the interrupts 
> > disabled longer than necessary.
> I know it is the third time for this patch. Yet it is in the RFC series 
> again.
So? RFC does not mean you have to ignore previous comments... You could 
have at least acknowledge my points...
> In this series I think I need interrupts locked until I start time 
> accounting for hypervisor. Time accounting is started by `tacc_head()` 
> function. I prefer to have it called from C, because it is more 
> convenient and obvious for those who are less familiar with the ARM code.
> > Resending the patch without things addressed is only going to make it 
> > worst.
> I'm still convinced the patch would improve interrupt latency for high 
> interrupt rate use cases.
> But I understand that I have no experiment to prove the effect, so I'm 
> not willing to push through the patch.
The only thing I ask is justification in your commit message rather than 
throwing things and expecting the reviewer to understand why you do 
that. I would recommend to refresh yourself how to submit a patch series 
[1].
> Also, I have a question to you about another aspect of this patch. In 
> the function `enter_hypervisor_head()` there is a check for a disabled 
> workaround and turning it on. If we have the interrupts enabled until 
> there, we have good chances to go through the interrupt processing 
> `do_IRQ()` before WA enabled. Is it still OK?
Hmmm, that's correct.

Cheers,

[1] https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel