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@xxxxxxxxx. 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. 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>