In pseries_migration_partition(), loop until the memory transfer is
complete. This way the calling drmgr process will not exit earlier,
allowing callbacks to be run only once the migration is fully completed.
If reading the VASI state is done after the hypervisor has completed the
migration, the HCALL is returning H_PARAMETER. We can safely assume that
the memory transfer is achieved if this happens.
This will also allow to manage the NMI watchdog state in the next commits.
Reviewed-by: Nathan Lynch <[email protected]>
Signed-off-by: Laurent Dufour <[email protected]>
---
arch/powerpc/platforms/pseries/mobility.c | 42 +++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 78f3f74c7056..907a779074d6 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -427,6 +427,43 @@ static int wait_for_vasi_session_suspending(u64 handle)
return ret;
}
+static void wait_for_vasi_session_completed(u64 handle)
+{
+ unsigned long state = 0;
+ int ret;
+
+ pr_info("waiting for memory transfert to complete...\n");
+
+ /*
+ * Wait for transition from H_VASI_RESUMED to H_VASI_COMPLETED.
+ */
+ while (true) {
+ ret = poll_vasi_state(handle, &state);
+
+ /*
+ * If the memory transfer is already complete and the migration
+ * has been cleaned up by the hypervisor, H_PARAMETER is return,
+ * which is translate in EINVAL by poll_vasi_state().
+ */
+ if (ret == -EINVAL || (!ret && state == H_VASI_COMPLETED)) {
+ pr_info("memory transfert completed.\n");
+ break;
+ }
+
+ if (ret) {
+ pr_err("H_VASI_STATE return error (%d)\n", ret);
+ break;
+ }
+
+ if (state != H_VASI_RESUMED) {
+ pr_err("unexpected H_VASI_STATE result %lu\n", state);
+ break;
+ }
+
+ msleep(500);
+ }
+}
+
static void prod_single(unsigned int target_cpu)
{
long hvrc;
@@ -673,9 +710,10 @@ static int pseries_migrate_partition(u64 handle)
vas_migration_handler(VAS_SUSPEND);
ret = pseries_suspend(handle);
- if (ret == 0)
+ if (ret == 0) {
post_mobility_fixup();
- else
+ wait_for_vasi_session_completed(handle);
+ } else
pseries_cancel_migration(handle, ret);
vas_migration_handler(VAS_RESUME);
--
2.36.1
Excerpts from Laurent Dufour's message of June 27, 2022 11:53 pm:
> In pseries_migration_partition(), loop until the memory transfer is
> complete. This way the calling drmgr process will not exit earlier,
> allowing callbacks to be run only once the migration is fully completed.
>
> If reading the VASI state is done after the hypervisor has completed the
> migration, the HCALL is returning H_PARAMETER. We can safely assume that
> the memory transfer is achieved if this happens.
>
> This will also allow to manage the NMI watchdog state in the next commits.
>
> Reviewed-by: Nathan Lynch <[email protected]>
> Signed-off-by: Laurent Dufour <[email protected]>
> ---
> arch/powerpc/platforms/pseries/mobility.c | 42 +++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 78f3f74c7056..907a779074d6 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -427,6 +427,43 @@ static int wait_for_vasi_session_suspending(u64 handle)
> return ret;
> }
>
> +static void wait_for_vasi_session_completed(u64 handle)
> +{
> + unsigned long state = 0;
> + int ret;
> +
> + pr_info("waiting for memory transfert to complete...\n");
^ extra t (also below)
> +
> + /*
> + * Wait for transition from H_VASI_RESUMED to H_VASI_COMPLETED.
> + */
> + while (true) {
> + ret = poll_vasi_state(handle, &state);
> +
> + /*
> + * If the memory transfer is already complete and the migration
> + * has been cleaned up by the hypervisor, H_PARAMETER is return,
> + * which is translate in EINVAL by poll_vasi_state().
> + */
> + if (ret == -EINVAL || (!ret && state == H_VASI_COMPLETED)) {
> + pr_info("memory transfert completed.\n");
> + break;
> + }
> +
> + if (ret) {
> + pr_err("H_VASI_STATE return error (%d)\n", ret);
> + break;
> + }
> +
> + if (state != H_VASI_RESUMED) {
> + pr_err("unexpected H_VASI_STATE result %lu\n", state);
> + break;
> + }
> +
> + msleep(500);
Is 500 specified anywhere? Another caller uses 1000, and the other one
uses some backoff interval starting at 1ms...
> + }
> +}
> +
> static void prod_single(unsigned int target_cpu)
> {
> long hvrc;
> @@ -673,9 +710,10 @@ static int pseries_migrate_partition(u64 handle)
> vas_migration_handler(VAS_SUSPEND);
>
> ret = pseries_suspend(handle);
> - if (ret == 0)
> + if (ret == 0) {
> post_mobility_fixup();
> - else
> + wait_for_vasi_session_completed(handle);
If this wasn't required until later patches, maybe a comment about why
it's here? Could call it wait_for_migration() or similar too.
Looks okay though from my basic reading of PAPR.
Reviewed-by: Nicholas Piggin <[email protected]>
> + } else
> pseries_cancel_migration(handle, ret);
>
> vas_migration_handler(VAS_RESUME);
> --
> 2.36.1
>
>
Le 12/07/2022 à 03:33, Nicholas Piggin a écrit :
> Excerpts from Laurent Dufour's message of June 27, 2022 11:53 pm:
>> In pseries_migration_partition(), loop until the memory transfer is
>> complete. This way the calling drmgr process will not exit earlier,
>> allowing callbacks to be run only once the migration is fully completed.
>>
>> If reading the VASI state is done after the hypervisor has completed the
>> migration, the HCALL is returning H_PARAMETER. We can safely assume that
>> the memory transfer is achieved if this happens.
>>
>> This will also allow to manage the NMI watchdog state in the next commits.
>>
>> Reviewed-by: Nathan Lynch <[email protected]>
>> Signed-off-by: Laurent Dufour <[email protected]>
>> ---
>> arch/powerpc/platforms/pseries/mobility.c | 42 +++++++++++++++++++++--
>> 1 file changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>> index 78f3f74c7056..907a779074d6 100644
>> --- a/arch/powerpc/platforms/pseries/mobility.c
>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>> @@ -427,6 +427,43 @@ static int wait_for_vasi_session_suspending(u64 handle)
>> return ret;
>> }
>>
>> +static void wait_for_vasi_session_completed(u64 handle)
>> +{
>> + unsigned long state = 0;
>> + int ret;
>> +
>> + pr_info("waiting for memory transfert to complete...\n");
>
> ^ extra t (also below)
I tried to push one French word, but you caught it ;)
Will fix that and the other ones.
>> +
>> + /*
>> + * Wait for transition from H_VASI_RESUMED to H_VASI_COMPLETED.
>> + */
>> + while (true) {
>> + ret = poll_vasi_state(handle, &state);
>> +
>> + /*
>> + * If the memory transfer is already complete and the migration
>> + * has been cleaned up by the hypervisor, H_PARAMETER is return,
>> + * which is translate in EINVAL by poll_vasi_state().
>> + */
>> + if (ret == -EINVAL || (!ret && state == H_VASI_COMPLETED)) {
>> + pr_info("memory transfert completed.\n");
>> + break;
>> + }
>> +
>> + if (ret) {
>> + pr_err("H_VASI_STATE return error (%d)\n", ret);
>> + break;
>> + }
>> +
>> + if (state != H_VASI_RESUMED) {
>> + pr_err("unexpected H_VASI_STATE result %lu\n", state);
>> + break;
>> + }
>> +
>> + msleep(500);
>
> Is 500 specified anywhere? Another caller uses 1000, and the other one
> uses some backoff interval starting at 1ms...
This is a bit empiric, the idea is to wait for the overall memory transfer
to be done. There is no real need to interact immediately after the
operation is terminated, so I pick that value to not make too many Hcalls
just for that. From the test I did, that seems to be a reasonable choice.
>
>> + }
>> +}
>> +
>> static void prod_single(unsigned int target_cpu)
>> {
>> long hvrc;
>> @@ -673,9 +710,10 @@ static int pseries_migrate_partition(u64 handle)
>> vas_migration_handler(VAS_SUSPEND);
>>
>> ret = pseries_suspend(handle);
>> - if (ret == 0)
>> + if (ret == 0) {
>> post_mobility_fixup();
>> - else
>> + wait_for_vasi_session_completed(handle);
>
> If this wasn't required until later patches, maybe a comment about why
> it's here? Could call it wait_for_migration() or similar too.
>
> Looks okay though from my basic reading of PAPR.
>
> Reviewed-by: Nicholas Piggin <[email protected]>
Thanks Nick for reviewing this series.
>
>> + } else
>> pseries_cancel_migration(handle, ret);
>>
>> vas_migration_handler(VAS_RESUME);
>> --
>> 2.36.1
>>
>>