Re: [PATCH v9 5/5] x86/ioreq server: Synchronously reset outstanding p2m_ioreq_server entries when an ioreq server unmaps.

[Yu Zhang <yu.c.zhang@linux.intel.com>]

On 3/21/2017 9:49 PM, Paul Durrant wrote:
>> -----Original Message-----
> [snip]
>>>> +        if ( (first_gfn > 0) || (data->flags == 0 && rc == 0) )
>>>> +        {
>>>> +            struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>>> +
>>>> +            while ( read_atomic(&p2m->ioreq.entry_count) &&
>>>> +                    first_gfn <= p2m->max_mapped_pfn )
>>>> +            {
>>>> +                /* Iterate p2m table for 256 gfns each time. */
>>>> +                last_gfn = first_gfn + 0xff;
>>>> +
>>> Might be worth a comment here to sat that p2m_finish_type_change()
>> limits last_gfn appropriately because it kind of looks wrong to be blindly
>> calling it with first_gfn + 0xff. Or perhaps, rather than passing last_gfn, pass a
>> 'max_nr' parameter of 256 instead. Then you can drop last_gfn altogether. If
>> you prefer the parameters as they are then at least limit the scope of
>> last_gfn to this while loop.
>> Thanks for your comments, Paul. :)
>> Well, setting last_gfn with first_gfn+0xff looks a bit awkward. But why
>> using a 'max_nr' with a magic number, say 256, looks better? Or any
>> other benefits? :-)
>>
> Well, to my eyes calling it max_nr in the function would make it clear it's a limit rather than a definite count and then passing 256 in the call would make it clear that it is the chosen batch size.
>
> Does that make sense?

Sounds reasonable. Thanks! :-)
Yu
>    Paul
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel


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