Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751407AbdFXN0W (ORCPT ); Sat, 24 Jun 2017 09:26:22 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:36312 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751276AbdFXN0U (ORCPT ); Sat, 24 Jun 2017 09:26:20 -0400 Date: Sat, 24 Jun 2017 21:26:14 +0800 From: Wei Yang To: Rasmus Villemoes Cc: Michal Hocko , Andrew Morton , Vlastimil Babka , Hillf Danton , Johannes Weiner , Tetsuo Handa , Vinayak Menon , Xishi Qiu , Hao Lee , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/page_alloc.c: eliminate unsigned confusion in __rmqueue_fallback Message-ID: <20170624132440.GA40323@WeideMacBook-Pro.local> Reply-To: Wei Yang References: <20170621094344.GC22051@dhcp22.suse.cz> <20170621185529.2265-1-linux@rasmusvillemoes.dk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="1LKvkjL3sHcu1TtY" Content-Disposition: inline In-Reply-To: <20170621185529.2265-1-linux@rasmusvillemoes.dk> 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 Content-Length: 3308 Lines: 93 --1LKvkjL3sHcu1TtY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 21, 2017 at 08:55:28PM +0200, Rasmus Villemoes wrote: >Since current_order starts as MAX_ORDER-1 and is then only >decremented, the second half of the loop condition seems >superfluous. However, if order is 0, we may decrement current_order >past 0, making it UINT_MAX. This is obviously too subtle ([1], [2]). > >Since we need to add some comment anyway, change the two variables to >signed, making the counting-down for loop look more familiar, and >apparently also making gcc generate slightly smaller code. > >[1] https://lkml.org/lkml/2016/6/20/493 >[2] https://lkml.org/lkml/2017/6/19/345 > >Signed-off-by: Rasmus Villemoes >--- >Michal, something like this, perhaps? > >mm/page_alloc.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > >diff --git a/mm/page_alloc.c b/mm/page_alloc.c >index 2302f250d6b1..e656f4da9772 100644 >--- a/mm/page_alloc.c >+++ b/mm/page_alloc.c >@@ -2204,19 +2204,23 @@ static bool unreserve_highatomic_pageblock(const s= truct alloc_context *ac, > * list of requested migratetype, possibly along with other pages from th= e same > * block, depending on fragmentation avoidance heuristics. Returns true if > * fallback was found so that __rmqueue_smallest() can grab it. >+ * >+ * The use of signed ints for order and current_order is a deliberate >+ * deviation from the rest of this file, to make the for loop >+ * condition simpler. > */ > static inline bool >-__rmqueue_fallback(struct zone *zone, unsigned int order, int start_migra= tetype) >+__rmqueue_fallback(struct zone *zone, int order, int start_migratetype) > { > struct free_area *area; >- unsigned int current_order; >+ int current_order; > struct page *page; > int fallback_mt; > bool can_steal; >=20 > /* Find the largest possible block of pages in the other list */ > for (current_order =3D MAX_ORDER-1; >- current_order >=3D order && current_order <=3D MAX_ORDER-1; >+ current_order >=3D order; > --current_order) { > area =3D &(zone->free_area[current_order]); > fallback_mt =3D find_suitable_fallback(area, current_order, >--=20 >2.11.0 Looks nice. Why I didn't come up with this change. Acked-by: Wei Yang --=20 Wei Yang Help you, Help me --1LKvkjL3sHcu1TtY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJZTmh2AAoJEKcLNpZP5cTd/+gP/iWLJ7ShsIdTwi75GSS8hBoc tNTwyFOPSZgc1HkLsEimPp/fYQwbv95eT9d8WHxV95fhgvDyqIRsVLwFkKzxhe3J fkXySx7zer5CxVLluoewrEkKwM3rLRaEiyomAu4dV5IzXnaGp/oDb1jgU0ynl1QQ 6TP1sipFatoUCeWra6AeEQrwnfMrC2H4p44lOwNOw8e0kKLvksz+Ckg3Uihmbl+a qG88nZ26U9G1ggVR04MRLMZMifmv2lO2HdzvnHlBdbCTT71h0T5QUUzOB7K0g/sF JLC36Imx4Xz5TksddZPxW91Lqun1bUftrHoaBN+KiBHrWpKEWvJp/5/j8anO/3/a XHTMEEcx5mARAXek4d2F071bkgVGkn9KHZc4xgPXSZRseaKnlFSe0KsXqTM8116W AXCI1KJ5uiQ9U2NIjcs8cSQWC5xiyHTnr2uQh3mbibN6Tzt33ZxuyNzBniia57EV X2jj2nZRTJS8H8bscG6+6f7m0g/mJ2B7OGQHGx6jTo309HlG9VCzdvrt8e9AxLP2 05GN2N1ngBao2X2ph6YbnBLbim1QuX1OdLfioljR+scMTInB2pg/Tw8PGUfzWlEl nsXtU6AHpBl4b5ez1GTB5hcA7qLX4aD9gUjN9/EY8JbxQJEX/+cdBUeF2dto6j9c JfrCDoQNnJFSHJCHXOaE =5FEK -----END PGP SIGNATURE----- --1LKvkjL3sHcu1TtY--