Wei Wang wrote:
> @@ -164,6 +284,8 @@ static unsigned fill_balloon(struct virtio_balloon *vb,
> size_t num)
> break;
> }
>
> + if (use_sg && xb_set_page(vb, page, &pfn_min, &pfn_max) < 0)
Isn't this leaking "page" ?
> + break;
> balloon_page_push(&pages, page);
> }
>
> @@ -184,8 +307,12 @@ static unsigned fill_balloon(struct virtio_balloon *vb,
> size_t num)
>
> num_allocated_pages = vb->num_pfns;
> /* Did we get any? */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->inflate_vq);
> + if (vb->num_pfns) {
> + if (use_sg)
> + tell_host_sgs(vb, vb->inflate_vq, pfn_min, pfn_max);
Please describe why tell_host_sgs() can work without __GFP_DIRECT_RECLAIM
allocation,
for tell_host_sgs() is called with vb->balloon_lock mutex held.
> + else
> + tell_host(vb, vb->inflate_vq);
> + }
> mutex_unlock(&vb->balloon_lock);
>
> return num_allocated_pages;
> @@ -223,7 +353,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb,
> size_t num)
> page = balloon_page_dequeue(vb_dev_info);
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (use_sg) {
> + if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0)
Isn't this leaking "page" ?
If this is inside vb->balloon_lock mutex (isn't this?), xb_set_page() must not
use __GFP_DIRECT_RECLAIM allocation, for leak_balloon_sg_oom() will be blocked
on vb->balloon_lock mutex.
> + break;
> + } else {
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + }
> +
> list_add(&page->lru, &pages);
> vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
