[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/3] migration/ram: Modify the code comment of ram_save_ho
From: |
Peter Xu |
Subject: |
Re: [PATCH v3 1/3] migration/ram: Modify the code comment of ram_save_host_page() |
Date: |
Fri, 5 Mar 2021 08:59:23 -0500 |
On Fri, Mar 05, 2021 at 03:50:33PM +0800, 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: Kunkun Jiang <jiangkunkun@huawei.com>
> ---
> migration/ram.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 72143da0ac..a168da5cdd 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1970,15 +1970,13 @@ 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 RAMBlock
> *
> - * 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.
> + * 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.
It reads more like a shorter version of original comment.. Could you point out
what's the major difference? Since the original comment looks still good to me.
> *
> * Returns the number of pages written or negative on error
> *
> @@ -2002,7 +2000,7 @@ static int ram_save_host_page(RAMState *rs,
> PageSearchStatus *pss,
> }
>
> do {
> - /* Check the pages is dirty and if it is send it */
> + /* Check if the page is dirty and send it if it is */
This line fixes some English issues, it seems. Looks okay, but normally I
won't change it unless the wording was too weird, so as to keep the git record
or the original lines. Here the original looks still okay, no strong opinion.
Thanks,
> if (!migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
> pss->page++;
> continue;
> --
> 2.23.0
>
--
Peter Xu
[PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Kunkun Jiang, 2021/03/05
- Re: [PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Peter Xu, 2021/03/05
- Re: [PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Kunkun Jiang, 2021/03/08
- Re: [PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Peter Xu, 2021/03/08
- Re: [PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Kunkun Jiang, 2021/03/09
- Re: [PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Peter Xu, 2021/03/09
- Re: [PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting, Kunkun Jiang, 2021/03/09