Re: [PATCH v17 4/6] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: 2017-11-03 11:25:32
Wei Wang wrote:
quoted hunk
@@ -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); }
quoted hunk
@@ -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;
quoted hunk
@@ -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;
}