search for: leak_balloon_sg_oom

...->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; > }

...->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; > }

...FP_NOWAIT or 0. Though Michael might remove that GFP argument ( http://lkml.kernel.org/r/201710022344.JII17368.HQtLOMJOOSFFVF at I-love.SAKURA.ne.jp ). > > 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. > > OK. Since the preload() doesn't need too much memory (< 4K in total), > how about GFP_NOWAIT here? Maybe GFP_NOWAIT | __GFP_NOWARN ?

...FP_NOWAIT or 0. Though Michael might remove that GFP argument ( http://lkml.kernel.org/r/201710022344.JII17368.HQtLOMJOOSFFVF at I-love.SAKURA.ne.jp ). > > 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. > > OK. Since the preload() doesn't need too much memory (< 4K in total), > how about GFP_NOWAIT here? Maybe GFP_NOWAIT | __GFP_NOWARN ?

...e *page; > > struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; > > LIST_HEAD(pages); > > + bool use_sg = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_SG); > > You can pass use_sg as an argument to leak_balloon(). Then, you won't need > to define leak_balloon_sg_oom(). Since xbitmap allocation does not use > __GFP_DIRECT_RECLAIM, it is safe to reuse leak_balloon() for OOM path. > Just be sure to pass use_sg == false because memory allocation for use_sg == > true likely fails when called from OOM path. (But trying use_sg == true for > OOM path and t...

...s visible to balloon_page_dequeue(). > > If a system has 128GB of memory, and 127GB of memory was stored into > LIST_HEAD(pages) upon first fill_balloon() request, and somebody held > balloon_lock from OOM notifier path from out_of_memory() before > fill_balloon() holds balloon_lock, leak_balloon_sg_oom() finds that > no memory can be freed because balloon_page_enqueue() was never called, > and allows the caller of out_of_memory() to invoke the OOM killer despite > there is 127GB of memory which can be freed if fill_balloon() was able > to hold balloon_lock before leak_balloon_sg_oom()...

...ar leak_balloon() with VIRTIO_BALLOON_F_SG needs memory allocation + * for xbitmap, which is not suitable for the oom case. This function does not + * use xbitmap to chunk pages, so it can be used by oom notifier to deflate + * pages when VIRTIO_BALLOON_F_SG is negotiated. + */ +static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb) +{ + unsigned int n; + struct page *page; + struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; + struct virtqueue *vq = vb->deflate_vq; + LIST_HEAD(pages); + + mutex_lock(&vb->balloon_lock); + for (n = 0; n < oom_pages; n++) { + page = balloon_p...

...ar leak_balloon() with VIRTIO_BALLOON_F_SG needs memory allocation + * for xbitmap, which is not suitable for the oom case. This function does not + * use xbitmap to chunk pages, so it can be used by oom notifier to deflate + * pages when VIRTIO_BALLOON_F_SG is negotiated. + */ +static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb) +{ + unsigned int n; + struct page *page; + struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; + struct virtqueue *vq = vb->deflate_vq; + LIST_HEAD(pages); + + mutex_lock(&vb->balloon_lock); + for (n = 0; n < oom_pages; n++) { + page = balloon_p...

...ar leak_balloon() with VIRTIO_BALLOON_F_SG needs memory allocation + * for xbitmap, which is not suitable for the oom case. This function does not + * use xbitmap to chunk pages, so it can be used by oom notifier to deflate + * pages when VIRTIO_BALLOON_F_SG is negotiated. + */ +static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb) +{ + unsigned int n; + struct page *page; + struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; + struct virtqueue *vq = vb->deflate_vq; + LIST_HEAD(pages); + + mutex_lock(&vb->balloon_lock); + for (n = 0; n < oom_pages; n++) { + page = balloon_p...

...ar leak_balloon() with VIRTIO_BALLOON_F_SG needs memory allocation + * for xbitmap, which is not suitable for the oom case. This function does not + * use xbitmap to chunk pages, so it can be used by oom notifier to deflate + * pages when VIRTIO_BALLOON_F_SG is negotiated. + */ +static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb) +{ + unsigned int n; + struct page *page; + struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; + struct virtqueue *vq = vb->deflate_vq; + LIST_HEAD(pages); + + mutex_lock(&vb->balloon_lock); + for (n = 0; n < oom_pages; n++) { + page = balloon_p...

...? Yes, will make it: if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) { balloon_page_enqueue(..., page); break; } > > 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. OK. Since the preload() doesn't need too much memory (< 4K in total), how about GFP_NOWAIT here? Best, Wei

...ord the page in the xb, >>>>> wouldn't we rather send it across as a single page? >>>>> >>>> I think that we need to be able to fallback to !use_sg path when OOM. >>> I also have different thoughts: >>> >>> 1) For OOM, we have leak_balloon_sg_oom (oom has nothing to do with >>> fill_balloon), which does not use xbitmap to record pages, thus no >>> memory allocation. >>> >>> 2) If the memory is already under pressure, it is pointless to >>> continue inflating memory to the host. We need to give th...

...page(). > >> If we can't record the page in the xb, >> wouldn't we rather send it across as a single page? >> > I think that we need to be able to fallback to !use_sg path when OOM. I also have different thoughts: 1) For OOM, we have leak_balloon_sg_oom (oom has nothing to do with fill_balloon), which does not use xbitmap to record pages, thus no memory allocation. 2) If the memory is already under pressure, it is pointless to continue inflating memory to the host. We need to give thanks to the memory allocation failure reported by xbitmap, w...

...posioning issue reported by Michael S. Tsirkin v16->v17: 1) patch 1: please check the commit log there; 2) patch 3: included Michael S. Tsirkin patch to fix the potential deadlock issue; 3) patch 4: use BUG_ON if virtqueue_add_ returns error, which is expected never to happen; 4) patch 4: add leak_balloon_sg_oom, which is used in the oom case when VIRTIO_BALLOON_F_SG is in use; 5) patch 6: use config registers, instead of a vq, as the command channel between the host and guest; 6) patch 6: add the command sequence id support. v15->v16: 1) mm: stop reporting the free pfn range if the callback returns fa...

...posioning issue reported by Michael S. Tsirkin v16->v17: 1) patch 1: please check the commit log there; 2) patch 3: included Michael S. Tsirkin patch to fix the potential deadlock issue; 3) patch 4: use BUG_ON if virtqueue_add_ returns error, which is expected never to happen; 4) patch 4: add leak_balloon_sg_oom, which is used in the oom case when VIRTIO_BALLOON_F_SG is in use; 5) patch 6: use config registers, instead of a vq, as the command channel between the host and guest; 6) patch 6: add the command sequence id support. v15->v16: 1) mm: stop reporting the free pfn range if the callback returns fa...

...es, so it can be used by oom notifier to deflate > + * pages when VIRTIO_BALLOON_F_SG is negotiated. > + */ I guess we can live with this for now. Two things to consider - adding support for pre-allocating indirect buffers - sorting the internal page queue (how?) > +static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb) > +{ > + unsigned int n; > + struct page *page; > + struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; > + struct virtqueue *vq = vb->deflate_vq; > + LIST_HEAD(pages); > + > + mutex_lock(&vb->balloon_lock); > + for (n = 0;...

...es, so it can be used by oom notifier to deflate > + * pages when VIRTIO_BALLOON_F_SG is negotiated. > + */ I guess we can live with this for now. Two things to consider - adding support for pre-allocating indirect buffers - sorting the internal page queue (how?) > +static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb) > +{ > + unsigned int n; > + struct page *page; > + struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info; > + struct virtqueue *vq = vb->deflate_vq; > + LIST_HEAD(pages); > + > + mutex_lock(&vb->balloon_lock); > + for (n = 0;...

...posioning issue reported by Michael S. Tsirkin v16->v17: 1) patch 1: please check the commit log there; 2) patch 3: included Michael S. Tsirkin patch to fix the potential deadlock issue; 3) patch 4: use BUG_ON if virtqueue_add_ returns error, which is expected never to happen; 4) patch 4: add leak_balloon_sg_oom, which is used in the oom case when VIRTIO_BALLOON_F_SG is in use; 5) patch 6: use config registers, instead of a vq, as the command channel between the host and guest; 6) patch 6: add the command sequence id support. v15->v16: 1) mm: stop reporting the free pfn range if the callback returns fa...

...posioning issue reported by Michael S. Tsirkin v16->v17: 1) patch 1: please check the commit log there; 2) patch 3: included Michael S. Tsirkin patch to fix the potential deadlock issue; 3) patch 4: use BUG_ON if virtqueue_add_ returns error, which is expected never to happen; 4) patch 4: add leak_balloon_sg_oom, which is used in the oom case when VIRTIO_BALLOON_F_SG is in use; 5) patch 6: use config registers, instead of a vq, as the command channel between the host and guest; 6) patch 6: add the command sequence id support. v15->v16: 1) mm: stop reporting the free pfn range if the callback returns fa...

...posioning issue reported by Michael S. Tsirkin v16->v17: 1) patch 1: please check the commit log there; 2) patch 3: included Michael S. Tsirkin patch to fix the potential deadlock issue; 3) patch 4: use BUG_ON if virtqueue_add_ returns error, which is expected never to happen; 4) patch 4: add leak_balloon_sg_oom, which is used in the oom case when VIRTIO_BALLOON_F_SG is in use; 5) patch 6: use config registers, instead of a vq, as the command channel between the host and guest; 6) patch 6: add the command sequence id support. v15->v16: 1) mm: stop reporting the free pfn range if the callback returns fa...