Let's move the removal of the page from the balloon list into the single
caller, to remove the dependency on the PG_isolated flag and clarify
locking requirements.

We'll shuffle the operations a bit such that they logically make more sense
(e.g., remove from the list before clearing flags).

In balloon migration functions we can now move the balloon_page_finalize()
out of the balloon lock and perform the finalization just before dropping
the balloon reference.

Document that the page lock is currently required when modifying the
movability aspects of a page; hopefully we can soon decouple this from the
page lock.

Signed-off-by: David Hildenbrand <da...@redhat.com>
---
 arch/powerpc/platforms/pseries/cmm.c |  2 +-
 drivers/misc/vmw_balloon.c           |  3 +-
 drivers/virtio/virtio_balloon.c      |  4 +--
 include/linux/balloon_compaction.h   | 43 +++++++++++-----------------
 mm/balloon_compaction.c              |  3 +-
 5 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/cmm.c 
b/arch/powerpc/platforms/pseries/cmm.c
index 5f4037c1d7fe8..5e0a718d1be7b 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -532,7 +532,6 @@ static int cmm_migratepage(struct balloon_dev_info 
*b_dev_info,
 
        spin_lock_irqsave(&b_dev_info->pages_lock, flags);
        balloon_page_insert(b_dev_info, newpage);
-       balloon_page_delete(page);
        b_dev_info->isolated_pages--;
        spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
 
@@ -542,6 +541,7 @@ static int cmm_migratepage(struct balloon_dev_info 
*b_dev_info,
         */
        plpar_page_set_active(page);
 
+       balloon_page_finalize(page);
        /* balloon page list reference */
        put_page(page);
 
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index c817d8c216413..6653fc53c951c 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1778,8 +1778,7 @@ static int vmballoon_migratepage(struct balloon_dev_info 
*b_dev_info,
         * @pages_lock . We keep holding @comm_lock since we will need it in a
         * second.
         */
-       balloon_page_delete(page);
-
+       balloon_page_finalize(page);
        put_page(page);
 
        /* Inflate */
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 89da052f4f687..e299e18346a30 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -866,15 +866,13 @@ static int virtballoon_migratepage(struct 
balloon_dev_info *vb_dev_info,
        tell_host(vb, vb->inflate_vq);
 
        /* balloon's page migration 2nd step -- deflate "page" */
-       spin_lock_irqsave(&vb_dev_info->pages_lock, flags);
-       balloon_page_delete(page);
-       spin_unlock_irqrestore(&vb_dev_info->pages_lock, flags);
        vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
        set_page_pfns(vb, vb->pfns, page);
        tell_host(vb, vb->deflate_vq);
 
        mutex_unlock(&vb->balloon_lock);
 
+       balloon_page_finalize(page);
        put_page(page); /* balloon reference */
 
        return MIGRATEPAGE_SUCCESS;
diff --git a/include/linux/balloon_compaction.h 
b/include/linux/balloon_compaction.h
index 5ca2d56996201..b9f19da37b089 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -97,27 +97,6 @@ static inline void balloon_page_insert(struct 
balloon_dev_info *balloon,
        list_add(&page->lru, &balloon->pages);
 }
 
-/*
- * balloon_page_delete - delete a page from balloon's page list and clear
- *                      the page->private assignement accordingly.
- * @page    : page to be released from balloon's page list
- *
- * Caller must ensure the page is locked and the spin_lock protecting balloon
- * pages list is held before deleting a page from the balloon device.
- */
-static inline void balloon_page_delete(struct page *page)
-{
-       __ClearPageOffline(page);
-       __ClearPageMovable(page);
-       set_page_private(page, 0);
-       /*
-        * No touch page.lru field once @page has been isolated
-        * because VM is using the field.
-        */
-       if (!PageIsolated(page))
-               list_del(&page->lru);
-}
-
 /*
  * balloon_page_device - get the b_dev_info descriptor for the balloon device
  *                      that enqueues the given page.
@@ -141,12 +120,6 @@ static inline void balloon_page_insert(struct 
balloon_dev_info *balloon,
        list_add(&page->lru, &balloon->pages);
 }
 
-static inline void balloon_page_delete(struct page *page)
-{
-       __ClearPageOffline(page);
-       list_del(&page->lru);
-}
-
 static inline gfp_t balloon_mapping_gfp_mask(void)
 {
        return GFP_HIGHUSER;
@@ -154,6 +127,22 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
 
 #endif /* CONFIG_BALLOON_COMPACTION */
 
+/*
+ * balloon_page_finalize - prepare a balloon page that was removed from the
+ *                        balloon list for release to the page allocator
+ * @page: page to be released to the page allocator
+ *
+ * Caller must ensure that the page is locked.
+ */
+static inline void balloon_page_finalize(struct page *page)
+{
+       if (IS_ENABLED(CONFIG_BALLOON_COMPACTION)) {
+               __ClearPageMovable(page);
+               set_page_private(page, 0);
+       }
+       __ClearPageOffline(page);
+}
+
 /*
  * balloon_page_push - insert a page into a page list.
  * @head : pointer to list
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index fcb60233aa35d..ec176bdb8a78b 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -94,7 +94,8 @@ size_t balloon_page_list_dequeue(struct balloon_dev_info 
*b_dev_info,
                if (!trylock_page(page))
                        continue;
 
-               balloon_page_delete(page);
+               list_del(&page->lru);
+               balloon_page_finalize(page);
                __count_vm_event(BALLOON_DEFLATE);
                list_add(&page->lru, pages);
                unlock_page(page);
-- 
2.49.0


Reply via email to