From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755001AbdETAe4 convert rfc822-to-8bit (ORCPT ); Fri, 19 May 2017 20:34:56 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:12486 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752272AbdETAex (ORCPT ); Fri, 19 May 2017 20:34:53 -0400 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Fri, 19 May 2017 17:34:52 -0700 Subject: Re: [PATCH] x86/mm: synchronize pgd in vmemmap_free() To: =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , , CC: "Kirill A. Shutemov" , Andrew Morton , Ingo Molnar , Michal Hocko , Mel Gorman References: <1495216887-3175-1-git-send-email-jglisse@redhat.com> <1495216887-3175-2-git-send-email-jglisse@redhat.com> X-Nvconfidentiality: public From: John Hubbard Message-ID: <07058bfe-8b70-0d0f-24ce-2dc978fe347b@nvidia.com> Date: Fri, 19 May 2017 17:34:51 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1495216887-3175-2-git-send-email-jglisse@redhat.com> X-Originating-IP: [172.17.160.221] X-ClientProxiedBy: HQMAIL106.nvidia.com (172.18.146.12) To HQMAIL108.nvidia.com (172.18.146.13) Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jerome, On 05/19/2017 11:01 AM, Jérôme Glisse wrote: > When we free kernel virtual map we should synchronize p4d/pud for > all the pgds to avoid any stall entry in non canonical pgd. "any stale entry in the non-canonical pgd", is what I think you meant to type there. Also, it would be nice to clarify that commit description a bit: I'm not sure what is meant here by a "non-canonical pgd". Also, it seems like the reshuffling of the internals of sync_global_pgds() deserves at least some mention here. More below. > > Signed-off-by: Jérôme Glisse > Cc: Kirill A. Shutemov > Cc: Andrew Morton > Cc: Ingo Molnar > Cc: Michal Hocko > Cc: Mel Gorman > --- > arch/x86/mm/init_64.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > index ff95fe8..df753f8 100644 > --- a/arch/x86/mm/init_64.c > +++ b/arch/x86/mm/init_64.c > @@ -108,8 +108,6 @@ void sync_global_pgds(unsigned long start, unsigned long end) > BUILD_BUG_ON(pgd_none(*pgd_ref)); > p4d_ref = p4d_offset(pgd_ref, address); > > - if (p4d_none(*p4d_ref)) > - continue; > > spin_lock(&pgd_lock); > list_for_each_entry(page, &pgd_list, lru) { > @@ -123,12 +121,16 @@ void sync_global_pgds(unsigned long start, unsigned long end) > pgt_lock = &pgd_page_get_mm(page)->page_table_lock; > spin_lock(pgt_lock); > > - if (!p4d_none(*p4d_ref) && !p4d_none(*p4d)) > - BUG_ON(p4d_page_vaddr(*p4d) > - != p4d_page_vaddr(*p4d_ref)); > - > - if (p4d_none(*p4d)) > + if (p4d_none(*p4d_ref)) { > set_p4d(p4d, *p4d_ref); Is the intention really to set p4d to a zeroed *p4d_ref, or is that a mistake? > + } else { > + if (!p4d_none(*p4d_ref) && !p4d_none(*p4d)) I think the code needs to be somewhat restructured, but as it stands, the above !p4d_none(*p4d_ref) will always be true, because first part of the if/else checked for the opposite case: p4d_none(*p4d_ref). This is a side effect of moving that block of code. > + BUG_ON(p4d_page_vaddr(*p4d) > + != p4d_page_vaddr(*p4d_ref)); > + > + if (p4d_none(*p4d)) > + set_p4d(p4d, *p4d_ref); > + } > > spin_unlock(pgt_lock); > } > @@ -1024,6 +1026,7 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct) > void __ref vmemmap_free(unsigned long start, unsigned long end) > { > remove_pagetable(start, end, false); > + sync_global_pgds(start, end - 1); This does fix the HMM crash that I was seeing in hmm-next. thanks, John Hubbard NVIDIA > } > > #ifdef CONFIG_MEMORY_HOTREMOVE > -- > 2.4.11 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: email@kvack.org >