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

[Boris Ostrovsky]

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. So the question is --- would it make
sense to do all of this separately from the balloon driver?


-boris