Re: [Xen-devel] [RFC 1/3] xen/balloon: Allow allocating DMA buffers

[Boris Ostrovsky]

On 05/21/2018 01:32 PM, Oleksandr Andrushchenko wrote:
> On 05/21/2018 07:35 PM, Boris Ostrovsky wrote:
>> On 05/21/2018 01:40 AM, Oleksandr Andrushchenko wrote:
>>> On 05/19/2018 01:04 AM, Boris Ostrovsky wrote:
>>>> On 05/17/2018 04:26 AM, Oleksandr Andrushchenko wrote:
>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>>> A commit message would be useful.
>>> Sure, v1 will have it
>>>>> Signed-off-by: Oleksandr Andrushchenko
>>>>> <oleksandr_andrushchenko@xxxxxxxx>
>>>>>
>>>>> ÂÂÂÂÂÂ for (i = 0; i < nr_pages; i++) {
>>>>> -ÂÂÂÂÂÂÂ page = alloc_page(gfp);
>>>>> -ÂÂÂÂÂÂÂ if (page == NULL) {
>>>>> -ÂÂÂÂÂÂÂÂÂÂÂ nr_pages = i;
>>>>> -ÂÂÂÂÂÂÂÂÂÂÂ state = BP_EAGAIN;
>>>>> -ÂÂÂÂÂÂÂÂÂÂÂ break;
>>>>> +ÂÂÂÂÂÂÂ if (ext_pages) {
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂ page = ext_pages[i];
>>>>> +ÂÂÂÂÂÂÂ } else {
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂ page = alloc_page(gfp);
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂ if (page == NULL) {
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ nr_pages = i;
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ state = BP_EAGAIN;
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂ }
>>>>> ÂÂÂÂÂÂÂÂÂÂ }
>>>>> ÂÂÂÂÂÂÂÂÂÂ scrub_page(page);
>>>>> ÂÂÂÂÂÂÂÂÂÂ list_add(&page->lru, &pages);
>>>>> @@ -529,7 +565,7 @@ static enum bp_state
>>>>> decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>>>>> ÂÂÂÂÂÂ i = 0;
>>>>> ÂÂÂÂÂÂ list_for_each_entry_safe(page, tmp, &pages, lru) {
>>>>> ÂÂÂÂÂÂÂÂÂÂ /* XENMEM_decrease_reservation requires a GFN */
>>>>> -ÂÂÂÂÂÂÂ frame_list[i++] = xen_page_to_gfn(page);
>>>>> +ÂÂÂÂÂÂÂ frames[i++] = xen_page_to_gfn(page);
>>>>> ÂÂ Â #ifdef CONFIG_XEN_HAVE_PVMMU
>>>>> ÂÂÂÂÂÂÂÂÂÂ /*
>>>>> @@ -552,18 +588,22 @@ static enum bp_state
>>>>> decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>>>>> ÂÂ #endif
>>>>> ÂÂÂÂÂÂÂÂÂÂ list_del(&page->lru);
>>>>> ÂÂ -ÂÂÂÂÂÂÂ balloon_append(page);
>>>>> +ÂÂÂÂÂÂÂ if (!ext_pages)
>>>>> +ÂÂÂÂÂÂÂÂÂÂÂ balloon_append(page);
>>>> So what you are proposing is not really ballooning. You are just
>>>> piggybacking on existing interfaces, aren't you?
>>> Sort of. Basically I need to {increase|decrease}_reservation, not
>>> actually
>>> allocating ballooned pages.
>>> Do you think I can simply EXPORT_SYMBOL for
>>> {increase|decrease}_reservation?
>>> Any other suggestion?
>>
>> I am actually wondering how much of that code you end up reusing. You
>> pretty much create new code paths in both routines and common code ends
>> up being essentially the hypercall.
> Well, I hoped that it would be easier to maintain if I modify existing
> code
> to support both use-cases, but I am also ok to create new routines if
> this
> seems to be reasonable - please let me know
>> Â So the question is --- would it make
>> sense to do all of this separately from the balloon driver?
> This can be done, but which driver will host this code then? If we
> move from
> the balloon driver, then this could go to either gntdev or grant-table.
> What's your preference?

A separate module?

Is there any use for this feature outside of your zero-copy DRM driver?

-boris