From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932459AbdA0SJZ (ORCPT ); Fri, 27 Jan 2017 13:09:25 -0500 Received: from mail-pg0-f67.google.com ([74.125.83.67]:32788 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753271AbdA0SJN (ORCPT ); Fri, 27 Jan 2017 13:09:13 -0500 Date: Sat, 28 Jan 2017 02:09:03 +0800 From: Wei Yang To: Ingo Molnar Cc: Wei Yang , tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [Resend PATCH] x86/e820: break the loop when the region is less then current region Message-ID: <20170127180903.GA48696@WeideMacBook-Pro.local> Reply-To: Wei Yang References: <20170127024724.38506-1-richard.weiyang@gmail.com> <20170127084723.GD25162@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="IS0zKkzwUGydFO0o" Content-Disposition: inline In-Reply-To: <20170127084723.GD25162@gmail.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --IS0zKkzwUGydFO0o Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, Ingo Glad to see your comments. On Fri, Jan 27, 2017 at 09:47:23AM +0100, Ingo Molnar wrote: > >* Wei Yang wrote: > >> e820_all_mapped() iterates the e820 table to check whether a region is >> mapped or not. Since the e820 table is sorted, when the region is less t= han >> the current region, no need to continue the iteration. >>=20 >> The patch breaks the loop accordingly. >>=20 >> Signed-off-by: Wei Yang >> --- >> arch/x86/kernel/e820.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >>=20 >> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c >> index 90e8dde..f4fb197 100644 >> --- a/arch/x86/kernel/e820.c >> +++ b/arch/x86/kernel/e820.c >> @@ -86,10 +86,16 @@ int __init e820_all_mapped(u64 start, u64 end, unsig= ned type) >> for (i =3D 0; i < e820->nr_map; i++) { >> struct e820entry *ei =3D &e820->map[i]; >> =20 >> + /* Since the e820 table is sorted, when the region is less >> + * than the current region, break it. >> + */ >> + if (ei->addr >=3D end) >> + break; > >Please have a look at the relevant sections in Documentation/CodingStyle. = (And=20 >yes, this file violates it in a number of ways, but that's no reason to ad= d to the=20 >mess.) > I took a look in the Documentation/CodingStyle, looks the comment style is = not the preferred one. While I have checked this by scripts/checkpatch.pl, which prompts 0 error and 0 warning, so I thought it is fine. I would change this if you like. >But, more importantly, the reason I have not applied the patch before is t= hat=20 >while it's true that the e820 map is _eventually_ sorted, have you made ce= rtain=20 >that all calls to e820_all_mapped() are done when the map is already sorte= d? > I think so, if not those caller really need to make sure not to do this at that moment. The e820_all_mapped() can't work well if it is not sorted, even without this change. For example, we have two ranges [0x1000, 0x1FFF] and [0x2000, 0x2FFF] but t= hey are not sorted in e820. Then a range [0x1500, 0x2500] would be judge not all mapped, which actually is in range [0x1000, 0x2FFF]. BTW, I went through all the callers, and most of them are called from subsys_init() or arch_initcall() which is after setup_memory_map() called in setup_arch(). And two of them are called by e820_add_kernel_range() and efi_reserve_boot_services(), and both are after setup_memory_map(). >Thanks, > > Ingo --=20 Wei Yang Help you, Help me --IS0zKkzwUGydFO0o Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYi4y/AAoJEKcLNpZP5cTd2sIQAKaZfF1RCBnpkJBXIgSpXZCp zwnEZr2BkaEuN02L0z0m8Ydk3AFsgAZXdT4COdmT3H9S4o/o0uHkJKg+nzjD60OX 6SyHsjtCz66/ZzipotkZJpSTCayYSvp4W/dLXgi5FFDYXPwAShaGveEZTJtsEEww vZCLnI2pb/EW+VW69U2b71IZbcgkn1eep610fPBQu6IjLEEBtxOnAvq4vpMNFhBq u/SCPLPL+OAUjJhPKlcptHiL4XyzuPDNZz4FMCMpj/m1izr/e/jn0lMXdZ75m6qd OuAV95ZGeYGkHasnfWcX6r931iqvJR1v+SbXMLMJBYPUfMowuW+w8oJnwXTg0U12 Hk5iRklRNf/BQbyinoDGNpwp2AQ3Qlva2eAChgj1RcBaVLbjnOWXVeuI0xxv1rOf h7wvsLTmpgZ64LIGWlueDT+0Ter8f7NsYvlyWbWHtm3UMdwMv8qpG9QMYh9FUZAl 8CLiwUMk1kvKaAGD6MHxgkhGWbgZra0AqQIvsvLgDkkd8IvqVLIS68JOFBOCa7lQ 1tcnBuK+jBGBHb4kP9MZAJFgoLSiAKrqH9BQy4FTZ9F/gwR5LyzHs944lcnSuvuT Hy4OyvzvQORMpv1OGfOJDGUnGXlSycT2p/GmTrk7mzThGWQS7Zdvjz2MWXAoXw1C PNFMFd+EMYXj3rxl/YX2 =XUXO -----END PGP SIGNATURE----- --IS0zKkzwUGydFO0o--