Commit c7cdff0e authored by Michael S. Tsirkin's avatar Michael S. Tsirkin
Browse files

virtio_balloon: fix deadlock on OOM 



fill_balloon doing memory allocations under balloon_lock
can cause a deadlock when leak_balloon is called from
virtballoon_oom_notify and tries to take same lock.

To fix, split page allocation and enqueue and do allocations outside the lock.

Here's a detailed analysis of the deadlock by Tetsuo Handa:

In leak_balloon(), mutex_lock(&vb->balloon_lock) is called in order to
serialize against fill_balloon(). But in fill_balloon(),
alloc_page(GFP_HIGHUSER[_MOVABLE] | __GFP_NOMEMALLOC | __GFP_NORETRY) is
called with vb->balloon_lock mutex held. Since GFP_HIGHUSER[_MOVABLE]
implies __GFP_DIRECT_RECLAIM | __GFP_IO | __GFP_FS, despite __GFP_NORETRY
is specified, this allocation attempt might indirectly depend on somebody
else's __GFP_DIRECT_RECLAIM memory allocation. And such indirect
__GFP_DIRECT_RECLAIM memory allocation might call leak_balloon() via
virtballoon_oom_notify() via blocking_notifier_call_chain() callback via
out_of_memory() when it reached __alloc_pages_may_oom() and held oom_lock
mutex. Since vb->balloon_lock mutex is already held by fill_balloon(), it
will cause OOM lockup.

  Thread1                                       Thread2
    fill_balloon()
      takes a balloon_lock
      balloon_page_enqueue()
        alloc_page(GFP_HIGHUSER_MOVABLE)
          direct reclaim (__GFP_FS context)       takes a fs lock
            waits for that fs lock                  alloc_page(GFP_NOFS)
                                                      __alloc_pages_may_oom()
                                                        takes the oom_lock
                                                        out_of_memory()
                                                          blocking_notifier_call_chain()
                                                            leak_balloon()
                                                              tries to take that balloon_lock and deadlocks

Reported-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
parent bebc6082

drivers/virtio/virtio_balloon.c

+19 −5
Original line number Diff line number Diff line
@@ -143,16 +143,17 @@ static void set_page_pfns(struct virtio_balloon *vb, 


static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)

{

	struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;

	unsigned num_allocated_pages;

	unsigned num_pfns;

	struct page *page;

	LIST_HEAD(pages);



	/* We can only do one array worth at a time. */

	num = min(num, ARRAY_SIZE(vb->pfns));



	mutex_lock(&vb->balloon_lock);

	for (vb->num_pfns = 0; vb->num_pfns < num;

	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {

		struct page *page = balloon_page_enqueue(vb_dev_info);

	for (num_pfns = 0; num_pfns < num;

	     num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {

		struct page *page = balloon_page_alloc();



		if (!page) {

			dev_info_ratelimited(&vb->vdev->dev,
@@ -162,6 +163,19 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num) 
			msleep(200);

			break;

		}



		balloon_page_push(&pages, page);

	}



	mutex_lock(&vb->balloon_lock);



	vb->num_pfns = 0;



	while ((page = balloon_page_pop(&pages))) {

		balloon_page_enqueue(&vb->vb_dev_info, page);



		vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE;



		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);

		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;

		if (!virtio_has_feature(vb->vdev,

include/linux/balloon_compaction.h

+34 −1
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ 
#include <linux/gfp.h>

#include <linux/err.h>

#include <linux/fs.h>

#include <linux/list.h>



/*

 * Balloon device information descriptor.
@@ -67,7 +68,9 @@ struct balloon_dev_info { 
	struct inode *inode;

};



extern struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info);

extern struct page *balloon_page_alloc(void);

extern void balloon_page_enqueue(struct balloon_dev_info *b_dev_info,

				 struct page *page);

extern struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info);



static inline void balloon_devinfo_init(struct balloon_dev_info *balloon)
@@ -193,4 +196,34 @@ static inline gfp_t balloon_mapping_gfp_mask(void) 
}



#endif /* CONFIG_BALLOON_COMPACTION */



/*

 * balloon_page_push - insert a page into a page list.

 * @head : pointer to list

 * @page : page to be added

 *

 * Caller must ensure the page is private and protect the list.

 */

static inline void balloon_page_push(struct list_head *pages, struct page *page)

{

	list_add(&page->lru, pages);

}



/*

 * balloon_page_pop - remove a page from a page list.

 * @head : pointer to list

 * @page : page to be added

 *

 * Caller must ensure the page is private and protect the list.

 */

static inline struct page *balloon_page_pop(struct list_head *pages)

{

	struct page *page = list_first_entry_or_null(pages, struct page, lru);



	if (!page)

		return NULL;



	list_del(&page->lru);

	return page;

}

#endif /* _LINUX_BALLOON_COMPACTION_H */

mm/balloon_compaction.c

+21 −7
Original line number Diff line number Diff line
@@ -10,23 +10,38 @@ 
#include <linux/export.h>

#include <linux/balloon_compaction.h>



/*

 * balloon_page_alloc - allocates a new page for insertion into the balloon

 *			  page list.

 *

 * Driver must call it to properly allocate a new enlisted balloon page.

 * Driver must call balloon_page_enqueue before definitively removing it from

 * the guest system.  This function returns the page address for the recently

 * allocated page or NULL in the case we fail to allocate a new page this turn.

 */

struct page *balloon_page_alloc(void)

{

	struct page *page = alloc_page(balloon_mapping_gfp_mask() |

				       __GFP_NOMEMALLOC | __GFP_NORETRY);

	return page;

}

EXPORT_SYMBOL_GPL(balloon_page_alloc);



/*

 * balloon_page_enqueue - allocates a new page and inserts it into the balloon

 *			  page list.

 * @b_dev_info: balloon device descriptor where we will insert a new page to

 * @page: new page to enqueue - allocated using balloon_page_alloc.

 *

 * Driver must call it to properly allocate a new enlisted balloon page

 * Driver must call it to properly enqueue a new allocated balloon page

 * before definitively removing it from the guest system.

 * This function returns the page address for the recently enqueued page or

 * NULL in the case we fail to allocate a new page this turn.

 */

struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info)

void balloon_page_enqueue(struct balloon_dev_info *b_dev_info,

			  struct page *page)

{

	unsigned long flags;

	struct page *page = alloc_page(balloon_mapping_gfp_mask() |

				       __GFP_NOMEMALLOC | __GFP_NORETRY);

	if (!page)

		return NULL;



	/*

	 * Block others from accessing the 'page' when we get around to
@@ -39,7 +54,6 @@ struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info) 
	__count_vm_event(BALLOON_INFLATE);

	spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);

	unlock_page(page);

	return page;

}

EXPORT_SYMBOL_GPL(balloon_page_enqueue);

CRA Git | Maintained and supported by SUSTech CRA and CCSE