Re: [PATCH v6 1/3] ioreq: switch ioreq page allocation to vmap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.08.2026 16:19, Julian Vetter wrote:
> On 8/18/26 3:06 PM, Jan Beulich wrote:
>> On 20.04.2026 11:38, Julian Vetter wrote:
>>> @@ -333,7 +335,8 @@ bool is_ioreq_server_page(struct domain *d, const 
>>> struct page_info *page)
>>>   
>>>       FOR_EACH_IOREQ_SERVER(d, id, s)
>>>       {
>>> -        if ( (s->ioreq.page == page) || (s->bufioreq.page == page) )
>>> +        if ( (s->ioreq.va && vmap_to_page(s->ioreq.va) == page) ||
>>> +             (s->bufioreq.va && vmap_to_page(s->bufioreq.va) == page) )
>>>           {
>>>               found = true;
>>>               break;
>>
>> You mention in the description that some extra overhead is introduced. The
>> (generally) two page walks done here are particularly concerning. Since we
>> have a valid struct page_info * available here, I wonder if we shouldn't
>> aid this lookup by recording the VA in one of struct page_info's fields.
>> Afaics vmap() doesn't use any of the fields, so it should be relatively
>> easy to determine a field to use for this purpose. The more involved part
>> would then be to make sure the field (in other struct page_info instances)
>> is also properly different from any VA vmap() may return.
> 
> Hello Jan,
> 
> Thank you again for your feedback! I will wait then for Anthony's 
> decision regarding whether the multi-page ioreq support and the ioreq_t 
> growth should be combined into a single effort, before I proceed further 
> with a v7.
> 
> I just wanted to clarify one thing regarding the overhead I mentioned in 
> the first patch's commit message ("this change has a small overhead in 
> the common case"). Here, I was referring to vmap()/vunmap() replacing 
> map_domain_page_global(). Where map_domain_page_global() has a directmap 
> fast path. I didn't mean the overhead of the added vmap_to_page(). I 
> should maybe clarify this better in my next iteration's commit message.
> 
> On your suggestion to cache the VA in struct page_info to speed up the 
> vmap_to_page() lookups in is_ioreq_server_page(): I looked through the 
> tree, and that function currently has only one caller 
> sh_remove_all_mappings() (in xen/arch/x86/mm/shadow/common.c) and is 
> only reached in a failure case. Given that, and given how widely shared 
> and size-critical struct page_info is, I'm wondering whether it's really 
> worth touching it for the gain of not having to do the 2 lookups. What 
> do you think?

Hmm, indeed. Yet how would we prevent new uses from being hit? At least
a comment may want adding somewhere (where it's not too easy to overlook).

That said, why the mention of "size-critical" when I said "determine a
field", not "add a field"? (Really in different context I've suggested
the same as a possibility to George, for his ASI work.)

Jan