Re: Linux grant map/unmap improvement proposal (Draft B)

[David Vrabel <david.vrabel@citrix.com>]

On 15/10/14 18:45, Zoltan Kiss wrote:
>> Safe grant unmap
>> ----------------
>>
>> Grant references will only be unmapped when they are no longer in use.
>> i.e., the page reference count is one.
>>
>>      int gnttab_unmap_refs_async(struct gnttab_unmap_grant_ref
>> *unmap_ops,
>>          struct gnttab_unmap_grant_ref *kunmap_ops,
>>          struct page **pages, unsigned int count,
>>          void (*done)(void *data), void *data);
>>
>> The `gnttab_unmap_refs_async()` function will unmap the grant
>> references using the supplied unmap operations and call `done(data)`.
>> The grant unmap will only be done once all pages are no longer in use.
> I'm a bit confused about this function. I guess it checks the refcount
> before unmap. But then what does the done(data) function does?

It's needed to callback into the calling driver once the unmap is
eventually complete, so it can complete the request etc.

>> It shall run synchronously on the first attempt (this is expected to
>> be the most common case).  If any page is in use, it shall queue the
>> unmap request to be tried at a later time.
>
> Who will own this queue? The caller (e.g. blkback)? How often should it
> retry? That retry is triggered by a timer?

It will be common and managed the core grant table subsystem/module.

The retry will be a simple timer.  This is expected to be such a rare
event that anything more sophisticated isn't necessary.

>> Identifying foreign pages
>> -------------------------
>>
>> A new page flag is introduced: PG_foreign.  This will alias PG_pinned
>> so it does not require an additional bit.
>>
>> If PG_foreign is set then `page->private` contains the grant reference
>> and domid for this foreign page.  This information can only be packed
>> into an unsigned long on 64-bit platforms.  32-bit platforms will have
>> to allocate an additional structure to store the domid and gref.
>>
>> The aliasing of PG_foreign and PG_pinned is safe because:
>>
>> - Page table pages will never be foreign.
>> - Foreign pages shall have `p2m[P] & FOREIGN_FRAME_BIT`.
>>
>> The use of the private field is safe because:
>>
>> - The page is allocated by the balloon driver and thus it owns the
>>    private field.
>>
>> - The other fields in the union (ptl, slab_cache, and first_page) will
>>    not be used because the page is not used in a page table, slab or
>>    compound page.
>>
> This flag sounds similar to the flag used in classic for netback grant
> mapping. Would it be accepted in upstream? Aliasing PG_pinned would make
> sure of that?

This should be the case.

David