Re: [PATCH 3/3] grant-table: defer releasing pages acquired in a grant copy

["Jan Beulich" <JBeulich@suse.com>]

>>> On 22.01.15 at 15:39, <david.vrabel@citrix.com> wrote:
> On 22/01/15 14:34, Jan Beulich wrote:
>>>>> On 20.01.15 at 19:19, <david.vrabel@citrix.com> wrote:
>>> Acquiring a page for the source or destination of a grant copy is an
>>> expensive operation.  A common use case is for two adjacent grant copy
>>> ops to operate on either the same source or the same destination page.
>>>
>>> Instead of always acquiring and releasing destination and source pages
>>> for each operation, release the page once it is no longer valid for
>>> the next op.
>>>
>>> If either the source or destination domains changes both pages are
>>> released as it is unlikely that either will still be valid.
>>>
>>> XenServer's performance benchmarks show modest improvements in network
>>> receive throughput (netback uses grant copy in the guest Rx path) and
>>> no regressions in disk performamnce (using tapdisk3 which grant copies
>>> as the backend).
> [...]
>>> +static bool_t gnttab_copy_buf_valid(const struct gnttab_copy *op,
>>> +                                    const struct gnttab_copy_ptr *p,
>>> +                                    const struct gnttab_copy_buf *b,
>>> +                                    unsigned int gref_flag)
>>> +{
>>> +    if ( !b->virt )
>>> +        return 0;
>>> +    if ( op->flags & gref_flag )
>>> +        return b->have_grant && p->u.ref == b->ptr.u.ref;
>>> +    return p->u.gmfn == b->ptr.u.gmfn;
>>> +}
>> 
>> ... whether you wouldn't better fold the op and gref_flag parameters
>> into one, as they're used only once and only together.
> 
> Do you mean something like?
> 
> static bool_t gnttab_copy_buf_valid(const struct gnttab_copy_ptr *p,
>                                    const struct gnttab_copy_buf *b,
>                                    bool_t is_gref)

Yes.

Jan