Message ID | 20230209024435.3392916-1-yajun.deng@linux.dev |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp90404wrn; Wed, 8 Feb 2023 19:01:02 -0800 (PST) X-Google-Smtp-Source: AK7set98cb9LfbaPoA+nwzfd9LyC7ZW7mBiuEAr+Txq6znDRSP+B26qVRRtzNelCzdBvDUtxBStb X-Received: by 2002:a17:906:1011:b0:8af:11b5:fabd with SMTP id 17-20020a170906101100b008af11b5fabdmr3002456ejm.5.1675911661976; Wed, 08 Feb 2023 19:01:01 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675911661; cv=none; d=google.com; s=arc-20160816; b=RoNJF2DthofgVbxr4yzv0i1Zrd/jlfoJbowxol05mrFrF5nSFibIbgf2WsAx4nrOJ3 gVLo4es9fMbFxeWZojkaJ9gGZyoPYYkto0xsriDZS9QHBxpvx9mvnIQRsvVHvJGk2wh8 YmWT9e/axg2jDoaKBir4w9ch/Tu25W0IL01cxMdLG9b69OhxI3W2kNW4imMhxv4VO0pZ AgwU10SG+VgpBc+robjKo/nKt+9BiX7hpR92smNOob5yt007mmqL+XkD7qR3aEXJrBPE 9EBim1fxm38l/S2Zn8fIPBR5gOFcLEJGJWNV7HuKApjtRESaIx6Mpt4e53uUm7cJ2pE7 y5OQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=6j791U1wIUFSOI7G5OFn8eDWYHiEod5wKDWuChvQKYw=; b=hR4K3leg+r6wztjVdTruZ2SCyuJ4XH64b7uXehxoCOHOL/PmFZE3y7lQ+seWZc1aZT VkxykeAh4vUAii70gJaRNDriMrjybuHWxheQ6CplGK1kCF9CFT4hl6bG2spxCiEzVyJ6 SdZqTnRANlbk81vqObq/QuRsULhllqkRt0kJaK7j2eDAglMKuk2pQdW/Td5PJhuTNhfN l/I5JKz+nqU4OMGY7+gnJTNnXM4zSunmFNwtqC/9uQg2jOST9in9CNEL8bRmoeIbmAlg 8zo/iuP9f7Q08ob9wHwtmOzH5Yapw5PYI46QYqEZSfXhRQnLeDVrOj7NndiAhOTfluqL /xTw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=YxT2MBU6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m10-20020aa7c48a000000b004aab47a577bsi692007edq.65.2023.02.08.19.00.24; Wed, 08 Feb 2023 19:01:01 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=YxT2MBU6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232318AbjBICzg (ORCPT <rfc822;ivan.orlov0322@gmail.com> + 99 others); Wed, 8 Feb 2023 21:55:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40476 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232259AbjBICyg (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 8 Feb 2023 21:54:36 -0500 X-Greylist: delayed 416 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Wed, 08 Feb 2023 18:51:52 PST Received: from out-184.mta0.migadu.com (out-184.mta0.migadu.com [IPv6:2001:41d0:1004:224b::b8]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D27286BF for <linux-kernel@vger.kernel.org>; Wed, 8 Feb 2023 18:51:51 -0800 (PST) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1675910693; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=6j791U1wIUFSOI7G5OFn8eDWYHiEod5wKDWuChvQKYw=; b=YxT2MBU6ERJfONrIkOap8zG+cxkJFGKQtMQMMqerZX2JyQAMZFxUDz8IrRZnONqOUFWPrS MGCYEkvg0+xid3z3ybjMcByY3Z4pmwpk5E3q5l0ZGTMS5VdXUPrZvwGrBSiF6r7xtE0D0B WGMhQb3Qe+2Aus6swE/JXS+/qAghOns= From: Yajun Deng <yajun.deng@linux.dev> To: akpm@linux-foundation.org Cc: ziy@nvidia.com, mgorman@techsingularity.net, david@redhat.com, vbabka@suse.cz, rppt@linux.ibm.com, osalvador@suse.de, rppt@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Yajun Deng <yajun.deng@linux.dev> Subject: [PATCH] mm/page_alloc: optimize the loop in find_suitable_fallback() Date: Thu, 9 Feb 2023 10:44:35 +0800 Message-Id: <20230209024435.3392916-1-yajun.deng@linux.dev> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757320746937949065?= X-GMAIL-MSGID: =?utf-8?q?1757320746937949065?= |
Series |
mm/page_alloc: optimize the loop in find_suitable_fallback()
|
|
Commit Message
Yajun Deng
Feb. 9, 2023, 2:44 a.m. UTC
There is no need to execute the next loop if it not return in the first
loop. So add a break at the end of the loop.
There are only three rows in fallbacks, so reduce the first index size
from MIGRATE_TYPES to MIGRATE_PCPTYPES.
Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
mm/page_alloc.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
Comments
On 2/9/23 03:44, Yajun Deng wrote: > There is no need to execute the next loop if it not return in the first > loop. So add a break at the end of the loop. > > There are only three rows in fallbacks, so reduce the first index size > from MIGRATE_TYPES to MIGRATE_PCPTYPES. > > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> Acked-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/page_alloc.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 1113483fa6c5..536e8d838fb5 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -2603,7 +2603,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order, > * > * The other migratetypes do not have fallbacks. > */ > -static int fallbacks[MIGRATE_TYPES][MIGRATE_PCPTYPES - 1] = { > +static int fallbacks[MIGRATE_PCPTYPES][MIGRATE_PCPTYPES - 1] = { > [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE }, > [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE }, > [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE }, > @@ -2861,7 +2861,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, > int i; > int fallback_mt; > > - if (area->nr_free == 0) > + if (area->nr_free == 0 || !migratetype_is_mergeable(migratetype)) Just curious, did you the check for extra safety or did you find (by running or code inspection) that this can be indeed called with a non-mergeable migratetype, and cause out of bounds access of the shrinked fallbacks array? BTW, I noticed the commment on migratetype_is_mergeable() contains: "See fallbacks[MIGRATE_TYPES][3] in page_alloc.c. " Should probably change it to e.g. "See fallbacks[][] array ..." so we don't have to keep it in exact sync... > return -1; > > *can_steal = false; > @@ -2873,11 +2873,10 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, > if (can_steal_fallback(order, migratetype)) > *can_steal = true; > > - if (!only_stealable) > - return fallback_mt; > - > - if (*can_steal) > + if (!only_stealable || *can_steal) > return fallback_mt; > + else > + break; > } > > return -1;
February 9, 2023 4:12 PM, "Vlastimil Babka" <vbabka@suse.cz> wrote: > On 2/9/23 03:44, Yajun Deng wrote: > >> There is no need to execute the next loop if it not return in the first >> loop. So add a break at the end of the loop. >> >> There are only three rows in fallbacks, so reduce the first index size >> from MIGRATE_TYPES to MIGRATE_PCPTYPES. >> >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > >> --- >> mm/page_alloc.c | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 1113483fa6c5..536e8d838fb5 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -2603,7 +2603,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order, >> * >> * The other migratetypes do not have fallbacks. >> */ >> -static int fallbacks[MIGRATE_TYPES][MIGRATE_PCPTYPES - 1] = { >> +static int fallbacks[MIGRATE_PCPTYPES][MIGRATE_PCPTYPES - 1] = { >> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE }, >> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE }, >> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE }, >> @@ -2861,7 +2861,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, >> int i; >> int fallback_mt; >> >> - if (area->nr_free == 0) >> + if (area->nr_free == 0 || !migratetype_is_mergeable(migratetype)) > > Just curious, did you the check for extra safety or did you find (by running > or code inspection) that this can be indeed called with a non-mergeable > migratetype, and cause out of bounds access of the shrinked fallbacks array? > No, I'm not sure if it is called with a non-mergeable migratetype. It is just for extra safety. > BTW, I noticed the commment on migratetype_is_mergeable() contains: > > "See fallbacks[MIGRATE_TYPES][3] in page_alloc.c. " > > Should probably change it to e.g. "See fallbacks[][] array ..." so we don't > have to keep it in exact sync... > Yes, this comment should be changed. So do I need to submit a v2 patch? >> return -1; >> >> *can_steal = false; >> @@ -2873,11 +2873,10 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, >> if (can_steal_fallback(order, migratetype)) >> *can_steal = true; >> >> - if (!only_stealable) >> - return fallback_mt; >> - >> - if (*can_steal) >> + if (!only_stealable || *can_steal) >> return fallback_mt; >> + else >> + break; >> } >> >> return -1;
On 2/9/23 09:44, Yajun Deng wrote: > February 9, 2023 4:12 PM, "Vlastimil Babka" <vbabka@suse.cz> wrote: > >> On 2/9/23 03:44, Yajun Deng wrote: >> >>> There is no need to execute the next loop if it not return in the first >>> loop. So add a break at the end of the loop. >>> >>> There are only three rows in fallbacks, so reduce the first index size >>> from MIGRATE_TYPES to MIGRATE_PCPTYPES. >>> >>> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> >> >> Acked-by: Vlastimil Babka <vbabka@suse.cz> >> >>> --- >>> mm/page_alloc.c | 11 +++++------ >>> 1 file changed, 5 insertions(+), 6 deletions(-) >>> >>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>> index 1113483fa6c5..536e8d838fb5 100644 >>> --- a/mm/page_alloc.c >>> +++ b/mm/page_alloc.c >>> @@ -2603,7 +2603,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order, >>> * >>> * The other migratetypes do not have fallbacks. >>> */ >>> -static int fallbacks[MIGRATE_TYPES][MIGRATE_PCPTYPES - 1] = { >>> +static int fallbacks[MIGRATE_PCPTYPES][MIGRATE_PCPTYPES - 1] = { >>> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE }, >>> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE }, >>> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE }, >>> @@ -2861,7 +2861,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, >>> int i; >>> int fallback_mt; >>> >>> - if (area->nr_free == 0) >>> + if (area->nr_free == 0 || !migratetype_is_mergeable(migratetype)) >> >> Just curious, did you the check for extra safety or did you find (by running >> or code inspection) that this can be indeed called with a non-mergeable >> migratetype, and cause out of bounds access of the shrinked fallbacks array? >> > > No, I'm not sure if it is called with a non-mergeable migratetype. > It is just for extra safety. OK, I agree with that. >> BTW, I noticed the commment on migratetype_is_mergeable() contains: >> >> "See fallbacks[MIGRATE_TYPES][3] in page_alloc.c. " >> >> Should probably change it to e.g. "See fallbacks[][] array ..." so we don't >> have to keep it in exact sync... >> > > Yes, this comment should be changed. > So do I need to submit a v2 patch? Please do, with my acked-by. >>> return -1; >>> >>> *can_steal = false; >>> @@ -2873,11 +2873,10 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, >>> if (can_steal_fallback(order, migratetype)) >>> *can_steal = true; >>> >>> - if (!only_stealable) >>> - return fallback_mt; >>> - >>> - if (*can_steal) >>> + if (!only_stealable || *can_steal) >>> return fallback_mt; >>> + else >>> + break; >>> } >>> >>> return -1;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 1113483fa6c5..536e8d838fb5 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2603,7 +2603,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order, * * The other migratetypes do not have fallbacks. */ -static int fallbacks[MIGRATE_TYPES][MIGRATE_PCPTYPES - 1] = { +static int fallbacks[MIGRATE_PCPTYPES][MIGRATE_PCPTYPES - 1] = { [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE }, [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE }, [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE }, @@ -2861,7 +2861,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, int i; int fallback_mt; - if (area->nr_free == 0) + if (area->nr_free == 0 || !migratetype_is_mergeable(migratetype)) return -1; *can_steal = false; @@ -2873,11 +2873,10 @@ int find_suitable_fallback(struct free_area *area, unsigned int order, if (can_steal_fallback(order, migratetype)) *can_steal = true; - if (!only_stealable) - return fallback_mt; - - if (*can_steal) + if (!only_stealable || *can_steal) return fallback_mt; + else + break; } return -1;