diff mbox series

[1/3] migration/ram: Modify the code comment of ram_save_host_page()

Message ID 20210223021646.500-2-jiangkunkun@huawei.com
State New
Headers show
Series migration/ram: Some modifications about ram_save_host_page() | expand

Commit Message

Kunkun Jiang Feb. 23, 2021, 2:16 a.m. UTC
The ram_save_host_page() has been modified several times
since its birth. But the comment hasn't been modified as it should
be. It'd better to modify the comment to explain ram_save_host_page()
more clearly.

Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
---
 migration/ram.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

David Edmondson Feb. 24, 2021, 10:53 p.m. UTC | #1
On Tuesday, 2021-02-23 at 10:16:43 +08, Kunkun Jiang wrote:

> The ram_save_host_page() has been modified several times
> since its birth. But the comment hasn't been modified as it should
> be. It'd better to modify the comment to explain ram_save_host_page()
> more clearly.
>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
> ---
>  migration/ram.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 72143da0ac..fc49c3f898 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1970,15 +1970,16 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,
>  }
>  
>  /**
> - * ram_save_host_page: save a whole host page
> + * ram_save_host_page: save a whole host page or the rest of a block
>   *
> - * Starting at *offset send pages up to the end of the current host
> - * page. It's valid for the initial offset to point into the middle of
> - * a host page in which case the remainder of the hostpage is sent.
> - * Only dirty target pages are sent. Note that the host page size may
> - * be a huge page for this block.
> - * The saving stops at the boundary of the used_length of the block
> - * if the RAMBlock isn't a multiple of the host page size.
> + * Starting at pss->page send pages up to the end of the current host
> + * page or the boundary of used_length of the block (if the RAMBlock
> + * isn't a multiple of the host page size). The min one is selected.
> + * Only dirty target pages are sent.
> + *
> + * Note that the host page size may be a huge page for this block, it's
> + * valid for the initial offset to point into the middle of a host page
> + * in which case the remainder of the hostpage is sent.

How about:

* Send dirty pages between pss->page and either the end of that page or
* the used_length of the RAMBlock, whichever is smaller.
*
* Note that if the host page is a huge page, pss->page may be in the
* middle of that page.

>   *
>   * Returns the number of pages written or negative on error
>   *
> -- 
> 2.23.0

dme.
Kunkun Jiang Feb. 25, 2021, 8:16 a.m. UTC | #2
On 2021/2/25 6:53, David Edmondson wrote:
> On Tuesday, 2021-02-23 at 10:16:43 +08, Kunkun Jiang wrote:
>
>> The ram_save_host_page() has been modified several times
>> since its birth. But the comment hasn't been modified as it should
>> be. It'd better to modify the comment to explain ram_save_host_page()
>> more clearly.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
>> ---
>>   migration/ram.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 72143da0ac..fc49c3f898 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1970,15 +1970,16 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,
>>   }
>>   
>>   /**
>> - * ram_save_host_page: save a whole host page
>> + * ram_save_host_page: save a whole host page or the rest of a block
>>    *
>> - * Starting at *offset send pages up to the end of the current host
>> - * page. It's valid for the initial offset to point into the middle of
>> - * a host page in which case the remainder of the hostpage is sent.
>> - * Only dirty target pages are sent. Note that the host page size may
>> - * be a huge page for this block.
>> - * The saving stops at the boundary of the used_length of the block
>> - * if the RAMBlock isn't a multiple of the host page size.
>> + * Starting at pss->page send pages up to the end of the current host
>> + * page or the boundary of used_length of the block (if the RAMBlock
>> + * isn't a multiple of the host page size). The min one is selected.
>> + * Only dirty target pages are sent.
>> + *
>> + * Note that the host page size may be a huge page for this block, it's
>> + * valid for the initial offset to point into the middle of a host page
>> + * in which case the remainder of the hostpage is sent.
> How about:
>
> * Send dirty pages between pss->page and either the end of that page or
> * the used_length of the RAMBlock, whichever is smaller.
> *
> * Note that if the host page is a huge page, pss->page may be in the
> * middle of that page.

Thank you. It looks concise and comprehensive.

Best Regards

Kunkun Jiang

>>    *
>>    * Returns the number of pages written or negative on error
>>    *
>> -- 
>> 2.23.0
> dme.
diff mbox series

Patch

diff --git a/migration/ram.c b/migration/ram.c
index 72143da0ac..fc49c3f898 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1970,15 +1970,16 @@  static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,
 }
 
 /**
- * ram_save_host_page: save a whole host page
+ * ram_save_host_page: save a whole host page or the rest of a block
  *
- * Starting at *offset send pages up to the end of the current host
- * page. It's valid for the initial offset to point into the middle of
- * a host page in which case the remainder of the hostpage is sent.
- * Only dirty target pages are sent. Note that the host page size may
- * be a huge page for this block.
- * The saving stops at the boundary of the used_length of the block
- * if the RAMBlock isn't a multiple of the host page size.
+ * Starting at pss->page send pages up to the end of the current host
+ * page or the boundary of used_length of the block (if the RAMBlock
+ * isn't a multiple of the host page size). The min one is selected.
+ * Only dirty target pages are sent.
+ *
+ * Note that the host page size may be a huge page for this block, it's
+ * valid for the initial offset to point into the middle of a host page
+ * in which case the remainder of the hostpage is sent.
  *
  * Returns the number of pages written or negative on error
  *