RE: [PATCH V3 17/23] xen/ioreq: Introduce domain_has_ioreq_server()

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Oleksandr <olekstysh@xxxxxxxxx>
> Sent: 08 December 2020 16:57
> To: Paul Durrant <paul@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>; Oleksandr Tyshchenko 
> <oleksandr_tyshchenko@xxxxxxxx>; Stefano
> Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Volodymyr 
> Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; 
> George Dunlap
> <george.dunlap@xxxxxxxxxx>; Ian Jackson <iwj@xxxxxxxxxxxxxx>; Wei Liu 
> <wl@xxxxxxx>; Julien Grall
> <julien.grall@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH V3 17/23] xen/ioreq: Introduce domain_has_ioreq_server()
> 
> 
> Hi Paul.
> 
> 
> On 08.12.20 17:33, Oleksandr wrote:
> >
> > On 08.12.20 17:11, Jan Beulich wrote:
> >
> > Hi Jan
> >
> >> On 30.11.2020 11:31, Oleksandr Tyshchenko wrote:
> >>> --- a/xen/include/xen/ioreq.h
> >>> +++ b/xen/include/xen/ioreq.h
> >>> @@ -55,6 +55,20 @@ struct ioreq_server {
> >>>       uint8_t                bufioreq_handling;
> >>>   };
> >>>   +/*
> >>> + * This should only be used when d == current->domain and it's not
> >>> paused,
> >> Is the "not paused" part really relevant here? Besides it being rare
> >> that the current domain would be paused (if so, it's in the process
> >> of having all its vCPU-s scheduled out), does this matter at all?do
> >> any extra actionsdo any extra actions
> >
> > No, it isn't relevant, I will drop it.
> >
> >
> >>
> >> Apart from this the patch looks okay to me, but I'm not sure it
> >> addresses Paul's concerns. Iirc he had suggested to switch back to
> >> a list if doing a swipe over the entire array is too expensive in
> >> this specific case.
> > We would like to avoid to do any extra actions in
> > leave_hypervisor_to_guest() if possible.
> > But not only there, the logic whether we check/set
> > mapcache_invalidation variable could be avoided if a domain doesn't
> > use IOREQ server...
> 
> 
> Are you OK with this patch (common part of it)?

How much of a performance benefit is this? The array is small to simply 
counting the non-NULL entries should be pretty quick.

  Paul

> 
> 
> --
> Regards,
> 
> Oleksandr Tyshchenko