Re: [PATCH 3/3] mm: adjust rss counters for migration entiries

[KAMEZAWA Hiroyuki]

On Fri, 06 Jan 2012 21:38:56 +0400
Konstantin Khlebnikov <khlebnikov@xxxxxxxxxx> wrote:

> Memory migration fill pte with migration entry and it didn't update rss counters.
> Then it replace migration entry with new page (or old one if migration was failed).
> But between this two passes this pte can be unmaped, or task can fork child and
> it will get copy of this migration entry. Nobody account this into rss counters.
> 
> This patch properly adjust rss counters for migration entries in zap_pte_range()
> and copy_one_pte(). Thus we avoid extra atomic operations on migration fast-path.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxx>

It's better to show wheter this is a bug-fix or not in changelog.

IIUC, the bug-fix is the 1st harf of this patch + patch [2/3].
Your new bug-check code is in patch[1/3] and 2nd half of this patch.

I think it's better to do bug-fix 1st and add bug-check later.

So, could you reorder patches to bug-fix and new-bug-check ?

To the logic itself,
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
Please CC when you repost.



> ---
>  mm/memory.c |   37 ++++++++++++++++++++++++++++---------
>  1 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 829d437..2f96ffc 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -878,15 +878,24 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  			}
>  			if (likely(!non_swap_entry(entry)))
>  				rss[MM_SWAPENTS]++;
> -			else if (is_write_migration_entry(entry) &&
> -					is_cow_mapping(vm_flags)) {
> -				/*
> -				 * COW mappings require pages in both parent
> -				 * and child to be set to read.
> -				 */
> -				make_migration_entry_read(&entry);
> -				pte = swp_entry_to_pte(entry);
> -				set_pte_at(src_mm, addr, src_pte, pte);
> +			else if (is_migration_entry(entry)) {
> +				page = migration_entry_to_page(entry);
> +
> +				if (PageAnon(page))
> +					rss[MM_ANONPAGES]++;
> +				else
> +					rss[MM_FILEPAGES]++;
> +
> +				if (is_write_migration_entry(entry) &&
> +				    is_cow_mapping(vm_flags)) {
> +					/*
> +					 * COW mappings require pages in both
> +					 * parent and child to be set to read.
> +					 */
> +					make_migration_entry_read(&entry);
> +					pte = swp_entry_to_pte(entry);
> +					set_pte_at(src_mm, addr, src_pte, pte);
> +				}
>  			}
>  		}
>  		goto out_set_pte;
> @@ -1191,6 +1200,16 @@ again:
>  
>  			if (!non_swap_entry(entry))
>  				rss[MM_SWAPENTS]--;
> +			else if (is_migration_entry(entry)) {
> +				struct page *page;
> +
> +				page = migration_entry_to_page(entry);
> +
> +				if (PageAnon(page))
> +					rss[MM_ANONPAGES]--;
> +				else
> +					rss[MM_FILEPAGES]--;
> +			}
>  			if (unlikely(!free_swap_and_cache(entry)))
>  				print_bad_pte(vma, addr, ptent, NULL);
>  		}
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/