Re: [PATCH] hv_balloon: Add the support of hibernation

[David Hildenbrand]

On 12.09.19 01:36, Dexuan Cui wrote:
> When hibernation is enabled, we must ignore the balloon up/down and
> hot-add requests from the host, if any.
> 
> Fow now, if people want to test hibernation, please blacklist hv_balloon
> or do not enable Dynamic Memory and Memory Resizing. See the comment in
> balloon_probe() for more info.
> 

Why do you even care about supporting hibernation? Can't you just pause
the VM in the hypervisor and continue to live a happy life? :)

> Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> ---
> 
> This patch is basically a pure Hyper-V specific change and it has a
> build dependency on the commit 271b2224d42f ("Drivers: hv: vmbus: Implement
> suspend/resume for VSC drivers for hibernation"), which is on Sasha Levin's
> Hyper-V tree's hyperv-next branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-next
> 
> I request this patch should go through Sasha's tree rather than the
> other tree(s).
> 
>  drivers/hv/hv_balloon.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 34bd735..7df0f67 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -24,6 +24,8 @@
>  
>  #include <linux/hyperv.h>
>  
> +#include <asm/mshyperv.h>
> +
>  #define CREATE_TRACE_POINTS
>  #include "hv_trace_balloon.h"
>  
> @@ -457,6 +459,7 @@ struct hot_add_wrk {
>  	struct work_struct wrk;
>  };
>  
> +static bool allow_hibernation;
>  static bool hot_add = true;
>  static bool do_hot_add;
>  /*
> @@ -1053,8 +1056,12 @@ static void hot_add_req(struct work_struct *dummy)
>  	else
>  		resp.result = 0;
>  
> -	if (!do_hot_add || (resp.page_count == 0))
> -		pr_err("Memory hot add failed\n");
> +	if (!do_hot_add || resp.page_count == 0) {
> +		if (!allow_hibernation)
> +			pr_err("Memory hot add failed\n");
> +		else
> +			pr_info("Ignore hot-add request!\n");
> +	}
>  
>  	dm->state = DM_INITIALIZED;
>  	resp.hdr.trans_id = atomic_inc_return(&trans_id);
> @@ -1509,6 +1516,11 @@ static void balloon_onchannelcallback(void *context)
>  			break;
>  
>  		case DM_BALLOON_REQUEST:
> +			if (allow_hibernation) {
> +				pr_info("Ignore balloon-up request!\n");
> +				break;
> +			}
> +
>  			if (dm->state == DM_BALLOON_UP)
>  				pr_warn("Currently ballooning\n");
>  			bal_msg = (struct dm_balloon *)recv_buffer;
> @@ -1518,6 +1530,11 @@ static void balloon_onchannelcallback(void *context)
>  			break;
>  
>  		case DM_UNBALLOON_REQUEST:
> +			if (allow_hibernation) {
> +				pr_info("Ignore balloon-down request!\n");
> +				break;
> +			}
> +
>  			dm->state = DM_BALLOON_DOWN;
>  			balloon_down(dm,
>  				 (struct dm_unballoon_request *)recv_buffer);
> @@ -1623,6 +1640,11 @@ static int balloon_connect_vsp(struct hv_device *dev)
>  	cap_msg.hdr.size = sizeof(struct dm_capabilities);
>  	cap_msg.hdr.trans_id = atomic_inc_return(&trans_id);
>  
> +	/*
> +	 * When hibernation (i.e. virtual ACPI S4 state) is enabled, the host
> +	 * currently still requires the bits to be set, so we have to add code
> +	 * to fail the host's hot-add and balloon up/down requests, if any.
> +	 */
>  	cap_msg.caps.cap_bits.balloon = 1;
>  	cap_msg.caps.cap_bits.hot_add = 1;
>  
> @@ -1672,6 +1694,24 @@ static int balloon_probe(struct hv_device *dev,
>  {
>  	int ret;
>  
> +#if 0

I am not sure if that's a good idea. Can't you base this series on
hv_is_hibernation_supported() ?

> +	/*
> +	 * The patch to implement hv_is_hibernation_supported() is going
> +	 * through the tip tree. For now, let's hardcode allow_hibernation
> +	 * to false to keep the current behavior of hv_balloon. If people
> +	 * want to test hibernation, please blacklist hv_balloon fow now
> +	 * or do not enable Dynamid Memory and Memory Resizing.
> +	 *
> +	 * We'll remove the conditional compilation as soon as
> +	 * hv_is_hibernation_supported() is available in the mainline tree.
> +	 */
> +	allow_hibernation = hv_is_hibernation_supported();
> +#else
> +	allow_hibernation = false;
> +#endif
> +	if (allow_hibernation)
> +		hot_add = false;
> +
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  	do_hot_add = hot_add;
>  #else
> @@ -1711,6 +1751,8 @@ static int balloon_probe(struct hv_device *dev,
>  	return 0;
>  
>  probe_error:
> +	dm_device.state = DM_INIT_ERROR;
> +	dm_device.thread  = NULL;
>  	vmbus_close(dev->channel);
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  	unregister_memory_notifier(&hv_memory_nb);
> @@ -1752,6 +1794,59 @@ static int balloon_remove(struct hv_device *dev)
>  	return 0;
>  }
>  
> +static int balloon_suspend(struct hv_device *hv_dev)
> +{
> +	struct hv_dynmem_device *dm = hv_get_drvdata(hv_dev);
> +
> +	tasklet_disable(&hv_dev->channel->callback_event);
> +
> +	cancel_work_sync(&dm->balloon_wrk.wrk);
> +	cancel_work_sync(&dm->ha_wrk.wrk);
> +
> +	if (dm->thread) {
> +		kthread_stop(dm->thread);
> +		dm->thread = NULL;
> +		vmbus_close(hv_dev->channel);
> +	}
> +
> +	tasklet_enable(&hv_dev->channel->callback_event);
> +
> +	return 0;
> +
> +}
> +
> +static int balloon_resume(struct hv_device *dev)
> +{
> +	int ret;
> +
> +	dm_device.state = DM_INITIALIZING;
> +
> +	ret = balloon_connect_vsp(dev);
> +
> +	if (ret != 0)
> +		goto out;
> +
> +	dm_device.thread =
> +		 kthread_run(dm_thread_func, &dm_device, "hv_balloon");
> +	if (IS_ERR(dm_device.thread)) {
> +		ret = PTR_ERR(dm_device.thread);
> +		dm_device.thread = NULL;
> +		goto close_channel;
> +	}
> +
> +	dm_device.state = DM_INITIALIZED;
> +	return 0;
> +close_channel:
> +	vmbus_close(dev->channel);
> +out:
> +	dm_device.state = DM_INIT_ERROR;
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +	unregister_memory_notifier(&hv_memory_nb);
> +	restore_online_page_callback(&hv_online_page);
> +#endif
> +	return ret;
> +}
> +
>  static const struct hv_vmbus_device_id id_table[] = {
>  	/* Dynamic Memory Class ID */
>  	/* 525074DC-8985-46e2-8057-A307DC18A502 */
> @@ -1766,6 +1861,8 @@ static int balloon_remove(struct hv_device *dev)
>  	.id_table = id_table,
>  	.probe =  balloon_probe,
>  	.remove =  balloon_remove,
> +	.suspend = balloon_suspend,
> +	.resume = balloon_resume,
>  	.driver = {
>  		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
> 


-- 

Thanks,

David / dhildenb