From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751357AbdEBHk5 (ORCPT ); Tue, 2 May 2017 03:40:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34714 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750762AbdEBHk4 (ORCPT ); Tue, 2 May 2017 03:40:56 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E405CC1CA93D Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=bhe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E405CC1CA93D Date: Tue, 2 May 2017 15:40:47 +0800 From: Baoquan He To: Ingo Molnar Cc: Yinghai Lu , Linux Kernel Mailing List , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , the arch/x86 maintainers , Kees Cook , Thomas Garnier , Andrew Morton , Yasuaki Ishimatsu , Jinbum Park , Dave Hansen , "Kirill A. Shutemov" , Dan Williams , Dave Young Subject: Re: [PATCH] x86/mm: Fix incorrect for loop count calculation in sync_global_pgds Message-ID: <20170502074047.GB2579@x1> References: <1493638874-4014-1-git-send-email-bhe@redhat.com> <20170502071837.GA2579@x1> <20170502072449.i755v4qtef3mibdb@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170502072449.i755v4qtef3mibdb@gmail.com> User-Agent: Mutt/1.7.0 (2016-08-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 02 May 2017 07:40:56 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/02/17 at 09:24am, Ingo Molnar wrote: > > * Baoquan He wrote: > > > On 05/01/17 at 03:37pm, Yinghai Lu wrote: > > > On Mon, May 1, 2017 at 4:41 AM, Baoquan He wrote: > > > > arch/x86/mm/init_64.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > > > > index 15173d3..dbf4f00 100644 > > > > --- a/arch/x86/mm/init_64.c > > > > +++ b/arch/x86/mm/init_64.c > > > > @@ -94,12 +94,14 @@ __setup("noexec32=", nonx32_setup); > > > > */ > > > > void sync_global_pgds(unsigned long start, unsigned long end) > > > > { > > > > - unsigned long address; > > > > + unsigned long address, address_next; > > > > > > > > - for (address = start; address <= end; address += PGDIR_SIZE) { > > > > + for (address = start; address <= end; address = address_next) { > > > > const pgd_t *pgd_ref = pgd_offset_k(address); > > > > struct page *page; > > > > > > > > + address_next = (address & PGDIR_MASK) + PGDIR_SIZE; > > > > + > > > > if (pgd_none(*pgd_ref)) > > > > continue; > > > > > > > > > > This one is better than V2. > > > > > > It would better if could rename address to addr as Ingo suggested. > > > > Thanks for your checking and suggestion, Yinghai. > > > > Both v1 and v2 are fine to me. As you said, code in v1 is easily > > understood, while v2 code is more compact, less line. The line of v1 is > > not more than 80. Maybe Ingo can help choose one which he likes better. > > Let's do the variant I suggested - that makes the loop self-contained ('continue' > would work as-is, etc.) and makes the code all around more readable. Sure, let me change as you said, will post after testing passed. Thanks a lot!