Re: [Xen-devel] [PATCH v7 08/12] x86/hvm/ioreq: maintain an array of ioreq servers rather than a list

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.09.17 at 14:12, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 26 September 2017 12:45
>> >>> On 26.09.17 at 12:55, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >> Sent: 25 September 2017 16:17
>> >> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
>> >> >>> On 18.09.17 at 17:31, <paul.durrant@xxxxxxxxxx> wrote:
>> >> > @@ -785,29 +797,27 @@ int hvm_get_ioreq_server_info(struct domain
>> >> >  void hvm_destroy_all_ioreq_servers(struct domain *d)
>> >> >  {
>> >> > -    struct hvm_ioreq_server *s, *next;
>> >> > +    struct hvm_ioreq_server *s;
>> >> > +    unsigned int id;
>> >> >
>> >> >      spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
>> >> >
>> >> >      /* No need to domain_pause() as the domain is being torn down */
>> >> >
>> >> > -    list_for_each_entry_safe ( s,
>> >> > -                               next,
>> >> > -                               &d->arch.hvm_domain.ioreq_server.list,
>> >> > -                               list_entry )
>> >> > +    FOR_EACH_IOREQ_SERVER(d, id, s)
>> >> >      {
>> >> > -        bool is_default = (s == d-
>> >arch.hvm_domain.default_ioreq_server);
>> >> > -
>> >> > -        hvm_ioreq_server_disable(s, is_default);
>> >> > -
>> >> > -        if ( is_default )
>> >> > -            d->arch.hvm_domain.default_ioreq_server = NULL;
>> >> > +        if ( !s )
>> >> > +            continue;
>> >> >
>> >> > -        list_del(&s->list_entry);
>> >> > +        hvm_ioreq_server_disable(s);
>> >> > +        hvm_ioreq_server_deinit(s);
>> >> >
>> >> > -        hvm_ioreq_server_deinit(s, is_default);
>> >> > +        ASSERT(d->arch.hvm_domain.ioreq_server.count);
>> >> > +        --d->arch.hvm_domain.ioreq_server.count;
>> >>
>> >> Seeing this - do you actually need the count as a separate field?
>> >> I.e. are there performance critical uses of it, where going through
>> >> the array would be too expensive? Most of the uses are just
>> >> ASSERT()s anyway.
>> >
>> > The specific case is in hvm_select_ioreq_server(). If there was no count
>> > then the array would have to be searched for the initial test.
>> 
>> And is this something that happens frequently, i.e. the
>> performance of which matters?
> 
> Yes, this is on the critical emulation path. I.e. it is a per-io call.

That's not answering the question, because you leave out implied
context: Is it a performance critical path to get here with no
ioreq server registers (i.e. count being zero)? After all if count is
non-zero, you get past the early-out conditional there anyway.

Jan


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