search for: xb_set_page

On Sat, Sep 30, 2017 at 12:05:52PM +0800, Wei Wang wrote: > +static inline void xb_set_page(struct virtio_balloon *vb, > + struct page *page, > + unsigned long *pfn_min, > + unsigned long *pfn_max) > +{ > + unsigned long pfn = page_to_pfn(page); > + > + *pfn_min = min(pfn, *pfn_min); > + *pfn_max = max(pfn, *pfn_max); > + xb_preload(GF...

On Sat, Sep 30, 2017 at 12:05:52PM +0800, Wei Wang wrote: > +static inline void xb_set_page(struct virtio_balloon *vb, > + struct page *page, > + unsigned long *pfn_min, > + unsigned long *pfn_max) > +{ > + unsigned long pfn = page_to_pfn(page); > + > + *pfn_min = min(pfn, *pfn_min); > + *pfn_max = max(pfn, *pfn_max); > + xb_preload(GF...

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 =...

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 =...

On 10/10/2017 09:09 PM, Tetsuo Handa wrote: > Wei Wang wrote: >>> And even if we could remove balloon_lock, you still cannot use >>> __GFP_DIRECT_RECLAIM at xb_set_page(). I think you will need to use >>> "whether it is safe to wait" flag from >>> "[PATCH] virtio: avoid possible OOM lockup at virtballoon_oom_notify()" . >> Without the lock being held, why couldn't we use __GFP_DIRECT_RECLAIM at >> xb_set_page()...

On 10/11/2017 10:26 AM, Tetsuo Handa wrote: > Wei Wang wrote: >> On 10/10/2017 09:09 PM, Tetsuo Handa wrote: >>> Wei Wang wrote: >>>>> And even if we could remove balloon_lock, you still cannot use >>>>> __GFP_DIRECT_RECLAIM at xb_set_page(). I think you will need to use >>>>> "whether it is safe to wait" flag from >>>>> "[PATCH] virtio: avoid possible OOM lockup at virtballoon_oom_notify()" . >>>> Without the lock being held, why couldn't we use __GFP_DIRECT_RECLAIM a...

On 11/03/2017 07:25 PM, Tetsuo Handa wrote: > 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" ? Right, thanks, will add __free_page(page) here. >> @@ -184,8 +307,12 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num) >> >> num_allocated_pages = vb->num_...

...this if necessary. Yes, please do so. Or maybe replace GFP_KERNEL with GFP_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 ?

...this if necessary. Yes, please do so. Or maybe replace GFP_KERNEL with GFP_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 ?

On 10/10/2017 07:08 PM, Tetsuo Handa wrote: > Wei Wang wrote: >> On 10/09/2017 11:20 PM, Michael S. Tsirkin wrote: >>> On Sat, Sep 30, 2017 at 12:05:52PM +0800, Wei Wang wrote: >>>> +static inline void xb_set_page(struct virtio_balloon *vb, >>>> + struct page *page, >>>> + unsigned long *pfn_min, >>>> + unsigned long *pfn_max) >>>> +{ >>>> + unsigned long pfn = page_to_pfn(page); >>>> + >>>> + *pfn_...

...t;> virtio_balloon *vb, size_t num) >>>>>> while ((page = balloon_page_pop(&pages))) { >>>>>> balloon_page_enqueue(&vb->vb_dev_info, page); >>>>>> + if (use_sg) { >>>>>> + if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) { >>>>>> + __free_page(page); >>>>>> + continue; >>>>>> + } >>>>>> + } else { >>>>>> + set_page_pfns(vb,...

...dhat.com> Cc: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp> --- drivers/virtio/virtio_balloon.c | 233 +++++++++++++++++++++++++++++++++--- include/uapi/linux/virtio_balloon.h | 1 + 2 files changed, 216 insertions(+), 18 deletions(-) RESEND ChangeLog: - fill_balloon(): move xb_set_page before balloon_page_enqueue diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index a1fb52c..d0b8ea0 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -32,6 +32,8 @@ #include <linux/mm.h> #include <linux/mount.h> #incl...

On Thursday, November 30, 2017 6:36 PM, Tetsuo Handa wrote: > Wei Wang wrote: > > +static inline int xb_set_page(struct virtio_balloon *vb, > > + struct page *page, > > + unsigned long *pfn_min, > > + unsigned long *pfn_max) > > +{ > > + unsigned long pfn = page_to_pfn(page); > > + int ret; > > + > > + *pfn_min = min(pfn, *pfn_min); &g...

...;>> @@ -173,8 +292,15 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num) >>> >>> while ((page = balloon_page_pop(&pages))) { >>> balloon_page_enqueue(&vb->vb_dev_info, page); >>> + if (use_sg) { >>> + if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) { >>> + __free_page(page); >>> + continue; >>> + } >>> + } else { >>> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page); >>> + } >> Is this the right behaviour? > I don...

...while (sg_len > sg_max_len) { + send_one_sg(vb, vq, sg_addr, sg_max_len); + sg_addr += sg_max_len; + sg_len -= sg_max_len; + } + send_one_sg(vb, vq, sg_addr, sg_len); + xb_zero(&vb->page_xb, sg_pfn_start, sg_pfn_end); + sg_pfn_start = sg_pfn_end + 1; + } +} + +static inline void xb_set_page(struct virtio_balloon *vb, + struct page *page, + unsigned long *pfn_min, + unsigned long *pfn_max) +{ + unsigned long pfn = page_to_pfn(page); + + *pfn_min = min(pfn, *pfn_min); + *pfn_max = max(pfn, *pfn_max); + xb_set_bit(&vb->page_xb, pfn); +} + static unsigne...

...+ */ + if (vq->num_free != virtqueue_get_vring_size(vq)) + kick_and_wait(vq, vb->acked); + + xb_clear_bit_range(&vb->page_xb, page_xb_start, page_xb_end); + return; + +err_out: + dev_warn(&vb->vdev->dev, "%s failure: %d\n", __func__, err); +} + +static inline void xb_set_page(struct virtio_balloon *vb, + struct page *page, + unsigned long *pfn_min, + unsigned long *pfn_max) +{ + unsigned long pfn = page_to_pfn(page); + + *pfn_min = min(pfn, *pfn_min); + *pfn_max = max(pfn, *pfn_max); + xb_preload(GFP_KERNEL); + xb_set_bit(&vb->page_xb,...

...mplement it in a style close to find_next_bit; - rename xb_find_next_zero_bit to xb_find_clear, and re-implement it in a stytle close to find_next_zero_bit; - separate the implementation of xb_find_set and xb_find_clear for the convenience of future updates. 3) patch 4: virtio-balloon - xb_set_page: change the way to call xb_ related APIs v18->v19: 1) patch 3: - xb_clear_bit_range and xb_find_next_bit will deal with range [start, end), where end is changed to be exclusive of the range. - add overflow checks at the end of xb_clear_bit_range and xb_find_next_bit - add overflow rel...

...mplement it in a style close to find_next_bit; - rename xb_find_next_zero_bit to xb_find_clear, and re-implement it in a stytle close to find_next_zero_bit; - separate the implementation of xb_find_set and xb_find_clear for the convenience of future updates. 3) patch 4: virtio-balloon - xb_set_page: change the way to call xb_ related APIs v18->v19: 1) patch 3: - xb_clear_bit_range and xb_find_next_bit will deal with range [start, end), where end is changed to be exclusive of the range. - add overflow checks at the end of xb_clear_bit_range and xb_find_next_bit - add overflow rel...

...e last few desc-s may not reach the batch size, but need a kick to + * notify the device to handle them. + */ + if (vq->num_free != virtqueue_get_vring_size(vq)) + kick_and_wait(vq, vb->acked); + + xb_clear_bit_range(&vb->page_xb, page_xb_start, page_xb_end); +} + +static inline int xb_set_page(struct virtio_balloon *vb, + struct page *page, + unsigned long *pfn_min, + unsigned long *pfn_max) +{ + unsigned long pfn = page_to_pfn(page); + int ret; + + *pfn_min = min(pfn, *pfn_min); + *pfn_max = max(pfn, *pfn_max); + + do { + ret = xb_preload_and_set_bit(&vb-...

...ait(vq, vb->acked); the actual trigger for kick is if we did not kick after the last send_one_desc. If kick is moved out of send_one_desc, you can test for that explicitly. > + > + xb_clear_bit_range(&vb->page_xb, page_xb_start, page_xb_end); > +} > + > +static inline int xb_set_page(struct virtio_balloon *vb, > + struct page *page, > + unsigned long *pfn_min, > + unsigned long *pfn_max) > +{ > + unsigned long pfn = page_to_pfn(page); > + int ret; > + > + *pfn_min = min(pfn, *pfn_min); > + *pfn_max = max(pfn, *pfn_max); >...