Re: [PATCH] virtio-balloon: Disable free page hinting/reporting if page poison is disabled

[David Hildenbrand <david@redhat.com>]

On 16.04.20 21:30, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> If we have free page hinting or page reporting enabled we should disable it
> if the pages are poisoned or initialized on free and we cannot notify the
> hypervisor.
> 

Please split that up into an actual fix (Fixes:) for free page reporting
and an optimization for free page hinting. Also, please document why we
are doing stuff.

Regarding the free page reporting part, I propose something like this instead


diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0ef16566c3f3..9b1e56da1e29 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1107,6 +1107,20 @@ static int virtballoon_restore(struct virtio_device *vdev)
 
 static int virtballoon_validate(struct virtio_device *vdev)
 {
+       /*
+        * Free page reporting relies on the fact that any access after
+        * pages were reported (esp. from the buddy) will result in them getting
+        * deflated automatically. In case we care about the page content, we
+        * need support from the hypervisor to provide us with the same page
+        * (poisoned) content we originally had in the page. Without
+        * VIRTIO_BALLOON_F_PAGE_POISON, we will receive either the original
+        * page or a zeroed page.
+        */
+       if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON) &&
+           !IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) &&
+           page_poisoning_enabled())
+               __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
+
        /* Tell the host whether we care about poisoned pages. */
        if (!want_init_on_free() &&
            (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) ||


> Fixes: 5d757c8d518d ("virtio-balloon: add support for providing free page reports to host")
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  drivers/virtio/virtio_balloon.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 95d9c2f0a7be..08bc86a6e468 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -1110,8 +1110,12 @@ static int virtballoon_validate(struct virtio_device *vdev)
>  	/* Tell the host whether we care about poisoned pages. */
>  	if (!want_init_on_free() &&
>  	    (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) ||
> -	     !page_poisoning_enabled()))
> +	     !page_poisoning_enabled())) {
>  		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
> +	} else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)) {
> +		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT);
> +		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
> +	}
>  
>  	__virtio_clear_bit(vdev, VIRTIO_F_IOMMU_PLATFORM);
>  	return 0;
> 


-- 
Thanks,

David / dhildenb