Re: [PATCH v2 05/23] mm/balloon_compaction: centralize adjust_managed_page_count() handling

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* David Hildenbrand (Red Hat) <david@kernel.org> [260115 04:21]:
> Let's centralize it, by allowing for the driver to enable this handling
> through a new flag (bool for now) in the balloon device info.
> 
> Note that we now adjust the counter when adding/removing a page into the
> balloon list: when removing a page to deflate it, it will now happen
> before the driver communicated with hypervisor, not afterwards.
> 
> This shouldn't make a difference in practice.
> 
> Signed-off-by: David Hildenbrand (Red Hat) <david@kernel.org>

For what it's worth,

Acked-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  arch/powerpc/platforms/pseries/cmm.c | 13 +------------
>  drivers/virtio/virtio_balloon.c      | 19 ++-----------------
>  include/linux/balloon_compaction.h   |  2 ++
>  mm/balloon_compaction.c              | 17 +++++++++++++++++
>  4 files changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 15f873f733a41..7fd8b3d7e7637 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -165,7 +165,6 @@ static long cmm_alloc_pages(long nr)
>  
>  		balloon_page_enqueue(&b_dev_info, page);
>  		atomic_long_inc(&loaned_pages);
> -		adjust_managed_page_count(page, -1);
>  		nr--;
>  	}
>  
> @@ -190,7 +189,6 @@ static long cmm_free_pages(long nr)
>  		if (!page)
>  			break;
>  		plpar_page_set_active(page);
> -		adjust_managed_page_count(page, 1);
>  		__free_page(page);
>  		atomic_long_dec(&loaned_pages);
>  		nr--;
> @@ -515,16 +513,6 @@ static int cmm_migratepage(struct balloon_dev_info *b_dev_info,
>  		return -EBUSY;
>  	}
>  
> -	/*
> -	 * When we migrate a page to a different zone, we have to fixup the
> -	 * count of both involved zones as we adjusted the managed page count
> -	 * when inflating.
> -	 */
> -	if (page_zone(page) != page_zone(newpage)) {
> -		adjust_managed_page_count(page, 1);
> -		adjust_managed_page_count(newpage, -1);
> -	}
> -
>  	/*
>  	 * activate/"deflate" the old page. We ignore any errors just like the
>  	 * other callers.
> @@ -551,6 +539,7 @@ static int cmm_init(void)
>  		return -EOPNOTSUPP;
>  
>  	balloon_devinfo_init(&b_dev_info);
> +	b_dev_info.adjust_managed_page_count = true;
>  	if (IS_ENABLED(CONFIG_BALLOON_COMPACTION))
>  		b_dev_info.migratepage = cmm_migratepage;
>  
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index df2756c071dae..15c1cf5fd249c 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -274,9 +274,6 @@ static unsigned int fill_balloon(struct virtio_balloon *vb, size_t num)
>  
>  		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
>  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> -		if (!virtio_has_feature(vb->vdev,
> -					VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> -			adjust_managed_page_count(page, -1);
>  		vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE;
>  	}
>  
> @@ -295,9 +292,6 @@ static void release_pages_balloon(struct virtio_balloon *vb,
>  	struct page *page, *next;
>  
>  	list_for_each_entry_safe(page, next, pages, lru) {
> -		if (!virtio_has_feature(vb->vdev,
> -					VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> -			adjust_managed_page_count(page, 1);
>  		list_del(&page->lru);
>  		put_page(page); /* balloon reference */
>  	}
> @@ -839,17 +833,6 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
>  	if (!mutex_trylock(&vb->balloon_lock))
>  		return -EAGAIN;
>  
> -	/*
> -	  * When we migrate a page to a different zone and adjusted the
> -	  * managed page count when inflating, we have to fixup the count of
> -	  * both involved zones.
> -	  */
> -	if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM) &&
> -	    page_zone(page) != page_zone(newpage)) {
> -		adjust_managed_page_count(page, 1);
> -		adjust_managed_page_count(newpage, -1);
> -	}
> -
>  	/* balloon's page migration 1st step  -- inflate "newpage" */
>  	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
>  	set_page_pfns(vb, vb->pfns, newpage);
> @@ -958,6 +941,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
>  	if (err)
>  		goto out_free_vb;
>  
> +	if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> +		vb->vb_dev_info.adjust_managed_page_count = true;
>  #ifdef CONFIG_BALLOON_COMPACTION
>  	vb->vb_dev_info.migratepage = virtballoon_migratepage;
>  #endif
> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
> index 7cfe48769239e..3109d3c43d306 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -56,6 +56,7 @@ struct balloon_dev_info {
>  	struct list_head pages;		/* Pages enqueued & handled to Host */
>  	int (*migratepage)(struct balloon_dev_info *, struct page *newpage,
>  			struct page *page, enum migrate_mode mode);
> +	bool adjust_managed_page_count;
>  };
>  
>  extern struct page *balloon_page_alloc(void);
> @@ -73,6 +74,7 @@ static inline void balloon_devinfo_init(struct balloon_dev_info *balloon)
>  	spin_lock_init(&balloon->pages_lock);
>  	INIT_LIST_HEAD(&balloon->pages);
>  	balloon->migratepage = NULL;
> +	balloon->adjust_managed_page_count = false;
>  }
>  
>  #ifdef CONFIG_BALLOON_COMPACTION
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index 5444c61bb9e76..fd9ec47cf4670 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -23,6 +23,8 @@ static void balloon_page_enqueue_one(struct balloon_dev_info *b_dev_info,
>  	BUG_ON(!trylock_page(page));
>  	balloon_page_insert(b_dev_info, page);
>  	unlock_page(page);
> +	if (b_dev_info->adjust_managed_page_count)
> +		adjust_managed_page_count(page, -1);
>  	__count_vm_event(BALLOON_INFLATE);
>  	inc_node_page_state(page, NR_BALLOON_PAGES);
>  }
> @@ -95,6 +97,8 @@ size_t balloon_page_list_dequeue(struct balloon_dev_info *b_dev_info,
>  			continue;
>  
>  		list_del(&page->lru);
> +		if (b_dev_info->adjust_managed_page_count)
> +			adjust_managed_page_count(page, 1);
>  		balloon_page_finalize(page);
>  		__count_vm_event(BALLOON_DEFLATE);
>  		list_add(&page->lru, pages);
> @@ -256,12 +260,25 @@ static int balloon_page_migrate(struct page *newpage, struct page *page,
>  
>  		balloon_page_insert(b_dev_info, newpage);
>  		__count_vm_event(BALLOON_MIGRATE);
> +
> +		if (b_dev_info->adjust_managed_page_count &&
> +		    page_zone(page) != page_zone(newpage)) {
> +			/*
> +			 * When we migrate a page to a different zone we
> +			 * have to fixup the count of both involved zones.
> +			 */
> +			adjust_managed_page_count(page, 1);
> +			adjust_managed_page_count(newpage, -1);
> +		}
>  		break;
>  	case -ENOENT:
>  		spin_lock_irqsave(&b_dev_info->pages_lock, flags);
>  
>  		/* Old page was deflated but new page not inflated. */
>  		__count_vm_event(BALLOON_DEFLATE);
> +
> +		if (b_dev_info->adjust_managed_page_count)
> +			adjust_managed_page_count(page, 1);
>  		break;
>  	default:
>  		return rc;
> -- 
> 2.52.0
>