public inbox for linux-hyperv@vger.kernel.org
 help / color / mirror / Atom feed
From: Mukesh R <mrathor@linux.microsoft.com>
To: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, longli@microsoft.com,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] mshv: Introduce hv_deposit_memory helper functions
Date: Thu, 29 Jan 2026 18:49:25 -0800	[thread overview]
Message-ID: <12bb86c5-51ea-6f5c-cb64-162383ae5454@linux.microsoft.com> (raw)
In-Reply-To: <aXqZSKmRdJMc6x5u@skinsburskii.localdomain>

On 1/28/26 15:18, Stanislav Kinsburskii wrote:
> On Tue, Jan 27, 2026 at 11:44:25AM -0800, Mukesh R wrote:
>> On 1/27/26 10:30, Stanislav Kinsburskii wrote:
>>> On Mon, Jan 26, 2026 at 06:06:23PM -0800, Mukesh R wrote:
>>>> On 1/25/26 14:41, Stanislav Kinsburskii wrote:
>>>>> On Fri, Jan 23, 2026 at 04:33:39PM -0800, Mukesh R wrote:
>>>>>> On 1/22/26 17:35, Stanislav Kinsburskii wrote:
>>>>>>> Introduce hv_deposit_memory_node() and hv_deposit_memory() helper
>>>>>>> functions to handle memory deposition with proper error handling.
>>>>>>>
>>>>>>> The new hv_deposit_memory_node() function takes the hypervisor status
>>>>>>> as a parameter and validates it before depositing pages. It checks for
>>>>>>> HV_STATUS_INSUFFICIENT_MEMORY specifically and returns an error for
>>>>>>> unexpected status codes.
>>>>>>>
>>>>>>> This is a precursor patch to new out-of-memory error codes support.
>>>>>>> No functional changes intended.
>>>>>>>
>>>>>>> Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
>>>>>>> ---
>>>>>>>      drivers/hv/hv_proc.c           |   22 ++++++++++++++++++++--
>>>>>>>      drivers/hv/mshv_root_hv_call.c |   25 +++++++++----------------
>>>>>>>      drivers/hv/mshv_root_main.c    |    3 +--
>>>>>>>      include/asm-generic/mshyperv.h |   10 ++++++++++
>>>>>>>      4 files changed, 40 insertions(+), 20 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
>>>>>>> index 80c66d1c74d5..c0c2bfc80d77 100644
>>>>>>> --- a/drivers/hv/hv_proc.c
>>>>>>> +++ b/drivers/hv/hv_proc.c
>>>>>>> @@ -110,6 +110,23 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>>>>>>>      }
>>>>>>>      EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
>>>>>>> +int hv_deposit_memory_node(int node, u64 partition_id,
>>>>>>> +			   u64 hv_status)
>>>>>>> +{
>>>>>>> +	u32 num_pages;
>>>>>>> +
>>>>>>> +	switch (hv_result(hv_status)) {
>>>>>>> +	case HV_STATUS_INSUFFICIENT_MEMORY:
>>>>>>> +		num_pages = 1;
>>>>>>> +		break;
>>>>>>> +	default:
>>>>>>> +		hv_status_err(hv_status, "Unexpected!\n");
>>>>>>> +		return -ENOMEM;
>>>>>>> +	}
>>>>>>> +	return hv_call_deposit_pages(node, partition_id, num_pages);
>>>>>>> +}
>>>>>>> +EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
>>>>>>> +
>>>>>>
>>>>>> Different hypercalls may want to deposit different number of pages in one
>>>>>> shot. As feature evolves, page sizes get mixed, we'd almost need that
>>>>>> flexibility. So, imo, either we just don't do this for now, or add num pages
>>>>>> parameter to be passed down.
>>>>>>
>>>>>
>>>>> What you do mean by "page sizes get mixed"?
>>>>> A helper to deposit num pages already exists: its
>>>>> hv_call_deposit_pages().
>>>>
>>>> My point, you are removing number of pages, and we may want to keep
>>>> that so one can quickly play around and change them.
>>>>
>>>> -                       ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>> -                                                   pt_id, 1);
>>>> +                       ret = hv_deposit_memory(pt_id, status);
>>>>
>>>> For example, in hv_call_initialize_partition() we may realize after
>>>> some analysis that depositing 2 pages or 4 pages is much better.
>>>>
>>>
>>> We have been using this 1-page deposit logic from the beginning. To
>>> change the number of pages, simply replace hv_deposit_memory with
>>> hv_call_deposit_pages and specify the desired number of pages.
>>
>> You could perhaps rename it to hv_deposit_page().
>>
> 
> Yes, this would be a good name, but unfortunately we can now receive
> statuses like HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY, where we need to
> deposit at least 8 consecutive pages. There is also another pair of
> status codes for required root pages, even when a guest partition-related
> hypercall is performed (see the next patch for details).
> This new helper is intended to cover all such cases, instead of branching
> for all these different cases in every function.

Got it, thanks.

> Thanks,
> Stanislav
> 
> 
>>> The proposed approach reduces code duplication and is less error-prone,
>>> as there are multiple error codes to handle. Consolidating the logic
>>> also makes the driver more robust.
>>>
>>>
>>> Thanks,  Stanislav
>>>
>>>>> Thanks,
>>>>> Stanislav
>>>>>
>>>>>> Thanks,
>>>>>> -Mukesh
>>>>>>
>>>>>>
>>>>>>
>>>>>>>      bool hv_result_oom(u64 status)
>>>>>>>      {
>>>>>>>      	switch (hv_result(status)) {
>>>>>>> @@ -155,7 +172,8 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
>>>>>>>      			}
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(node, hv_current_partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory_node(node, hv_current_partition_id,
>>>>>>> +					     status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -197,7 +215,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
>>>>>>>      			}
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(node, partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory_node(node, partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>>>>>>> index 58c5cbf2e567..06f2bac8039d 100644
>>>>>>> --- a/drivers/hv/mshv_root_hv_call.c
>>>>>>> +++ b/drivers/hv/mshv_root_hv_call.c
>>>>>>> @@ -123,8 +123,7 @@ int hv_call_create_partition(u64 flags,
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>>      		local_irq_restore(irq_flags);
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -					    hv_current_partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(hv_current_partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -151,7 +150,7 @@ int hv_call_initialize_partition(u64 partition_id)
>>>>>>>      			ret = hv_result_to_errno(status);
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -465,8 +464,7 @@ int hv_call_get_vp_state(u32 vp_index, u64 partition_id,
>>>>>>>      		}
>>>>>>>      		local_irq_restore(flags);
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -					    partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -525,8 +523,7 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
>>>>>>>      		}
>>>>>>>      		local_irq_restore(flags);
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -					    partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -573,7 +570,7 @@ static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>>>>      		local_irq_restore(flags);
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -722,8 +719,7 @@ hv_call_create_port(u64 port_partition_id, union hv_port_id port_id,
>>>>>>>      			ret = hv_result_to_errno(status);
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE, port_partition_id, 1);
>>>>>>> -
>>>>>>> +		ret = hv_deposit_memory(port_partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -776,8 +772,7 @@ hv_call_connect_port(u64 port_partition_id, union hv_port_id port_id,
>>>>>>>      			ret = hv_result_to_errno(status);
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -					    connection_partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(connection_partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -848,8 +843,7 @@ static int hv_call_map_stats_page2(enum hv_stats_object_type type,
>>>>>>>      			break;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -					    hv_current_partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(hv_current_partition_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	return ret;
>>>>>>> @@ -885,8 +879,7 @@ static int hv_call_map_stats_page(enum hv_stats_object_type type,
>>>>>>>      			return ret;
>>>>>>>      		}
>>>>>>> -		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -					    hv_current_partition_id, 1);
>>>>>>> +		ret = hv_deposit_memory(hv_current_partition_id, status);
>>>>>>>      		if (ret)
>>>>>>>      			return ret;
>>>>>>>      	} while (!ret);
>>>>>>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>>>>>>> index f4697497f83e..5fc572e31cd7 100644
>>>>>>> --- a/drivers/hv/mshv_root_main.c
>>>>>>> +++ b/drivers/hv/mshv_root_main.c
>>>>>>> @@ -264,8 +264,7 @@ static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
>>>>>>>      		if (!hv_result_oom(status))
>>>>>>>      			ret = hv_result_to_errno(status);
>>>>>>>      		else
>>>>>>> -			ret = hv_call_deposit_pages(NUMA_NO_NODE,
>>>>>>> -						    pt_id, 1);
>>>>>>> +			ret = hv_deposit_memory(pt_id, status);
>>>>>>>      	} while (!ret);
>>>>>>>      	args.status = hv_result(status);
>>>>>>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>>>>>>> index b73352a7fc9e..c8e8976839f8 100644
>>>>>>> --- a/include/asm-generic/mshyperv.h
>>>>>>> +++ b/include/asm-generic/mshyperv.h
>>>>>>> @@ -344,6 +344,7 @@ static inline bool hv_parent_partition(void)
>>>>>>>      }
>>>>>>>      bool hv_result_oom(u64 status);
>>>>>>> +int hv_deposit_memory_node(int node, u64 partition_id, u64 status);
>>>>>>>      int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
>>>>>>>      int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
>>>>>>>      int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
>>>>>>> @@ -353,6 +354,10 @@ static inline bool hv_root_partition(void) { return false; }
>>>>>>>      static inline bool hv_l1vh_partition(void) { return false; }
>>>>>>>      static inline bool hv_parent_partition(void) { return false; }
>>>>>>>      static inline bool hv_result_oom(u64 status) { return false; }
>>>>>>> +static inline int hv_deposit_memory_node(int node, u64 partition_id, u64 status)
>>>>>>> +{
>>>>>>> +	return -EOPNOTSUPP;
>>>>>>> +}
>>>>>>>      static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>>>>>>>      {
>>>>>>>      	return -EOPNOTSUPP;
>>>>>>> @@ -367,6 +372,11 @@ static inline int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u3
>>>>>>>      }
>>>>>>>      #endif /* CONFIG_MSHV_ROOT */
>>>>>>> +static inline int hv_deposit_memory(u64 partition_id, u64 status)
>>>>>>> +{
>>>>>>> +	return hv_deposit_memory_node(NUMA_NO_NODE, partition_id, status);
>>>>>>> +}
>>>>>>> +
>>>>>>>      #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>>>>>>>      u8 __init get_vtl(void);
>>>>>>>      #else
>>>>>>>
>>>>>>>
>>


  reply	other threads:[~2026-01-30  2:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-23  1:35 [PATCH 0/4] Improve Hyper-V memory deposit error handling Stanislav Kinsburskii
2026-01-23  1:35 ` [PATCH 1/4] mshv: Introduce hv_result_oom() helper function Stanislav Kinsburskii
2026-01-24  0:31   ` Mukesh R
2026-01-23  1:35 ` [PATCH 2/4] mshv: Introduce hv_deposit_memory helper functions Stanislav Kinsburskii
2026-01-24  0:33   ` Mukesh R
2026-01-25 22:41     ` Stanislav Kinsburskii
2026-01-27  2:06       ` Mukesh R
2026-01-27 18:30         ` Stanislav Kinsburskii
2026-01-27 19:44           ` Mukesh R
2026-01-28 23:18             ` Stanislav Kinsburskii
2026-01-30  2:49               ` Mukesh R [this message]
2026-01-23  1:35 ` [PATCH 3/4] mshv: Handle insufficient contiguous memory hypervisor status Stanislav Kinsburskii
2026-01-23  1:35 ` [PATCH 4/4] mshv: Handle insufficient root memory hypervisor statuses Stanislav Kinsburskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12bb86c5-51ea-6f5c-cb64-162383ae5454@linux.microsoft.com \
    --to=mrathor@linux.microsoft.com \
    --cc=decui@microsoft.com \
    --cc=haiyangz@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longli@microsoft.com \
    --cc=skinsburskii@linux.microsoft.com \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox