From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7879AC433DB for ; Thu, 11 Mar 2021 16:42:30 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EB42264FEC for ; Thu, 11 Mar 2021 16:42:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EB42264FEC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 56C898D02CE; Thu, 11 Mar 2021 11:42:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 51D4E8D02B2; Thu, 11 Mar 2021 11:42:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3A8288D02CE; Thu, 11 Mar 2021 11:42:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0104.hostedemail.com [216.40.44.104]) by kanga.kvack.org (Postfix) with ESMTP id 1D0AE8D02B2 for ; Thu, 11 Mar 2021 11:42:29 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id C7074180AD817 for ; Thu, 11 Mar 2021 16:42:28 +0000 (UTC) X-FDA: 77908161576.15.E683862 Received: from mail-io1-f42.google.com (mail-io1-f42.google.com [209.85.166.42]) by imf18.hostedemail.com (Postfix) with ESMTP id 4CC98200039F for ; Thu, 11 Mar 2021 16:42:29 +0000 (UTC) Received: by mail-io1-f42.google.com with SMTP id z13so22593387iox.8 for ; Thu, 11 Mar 2021 08:42:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=85YCALcXdYdlt5hBAqGVrxlQ+tcI4W++hP/xGI3GPIg=; b=SpSDrjjgCXXsmnhjzJldSUJKGFqiQr0RGU0laB1cgVYxXT2FezwKH/nHOuBwYECIs1 gzhw0HRKXoXZkalzPYjorZcx8wE/SJL7RIoLkM/p9CVXijfC1GcprA4gHqXu7zHnGlRh DKYiB5c1sTCKTlyWygqjhTY/SHoN+2EBM1sNZwEEQo+gSE09ad3mod5v5gHDNHyoZtkT yqnbs2TTSocz0ba5YP8+CTqMtaX2rT6Ap4Molv/UsGd6pa3UjJRGqycuwEp6K8il1MW9 jq4Ze9IYuWiLEZ5+17h0cIdC/3PnnUqBbb6AAxT5b22K7k15RAki5Vkl7f+g2vPadAV/ jYyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=85YCALcXdYdlt5hBAqGVrxlQ+tcI4W++hP/xGI3GPIg=; b=Sg6Iwur85IzcNPiueWLTLfQ/+OaVaHx5pnXw6N9YnAXXEDi+Sr9p82EubunG1MYxAO wLwA8SsymR3qK+cFlTZrOk/IoGSCkOijaeVUXFGPz6plEY78mFjCqvsB/qu+dhRF3xD5 UEtLTU5QJ6Q068H2ym1NwMDGZZL84H9y3DJgNeZwiHoFFF9Wg+Ndab8NTXAr7cxbR9o2 yGPfyhrF37vdXbeWk4RZakQ3HnS8R8/jVIyQvid6YInwFWqZLsujH4oS5B2snxsPzSSx XGBEJA+TjKYJyOBumys2CGj/i1xB5xY7iOZd3ZJ2i+mzPMW/F4UbZYfNU8DMC+2nY4mt B/vw== X-Gm-Message-State: AOAM5333OFQXDxKKjFfUdnG5U7K0HS36mDGnXQOKdtX8NAeq6022mC49 GUVtJK7mfvVzz6kc2n4JPZ0fyZpT7fDv1t+Vgr0= X-Google-Smtp-Source: ABdhPJzdwLWqA0vHm16sIX64k5YxxEfmprvL4krrXgLvdXiezVI8RFEpZV2UfL1vYB6W0T/6g2JAT5ZD696fPq803H4= X-Received: by 2002:a5d:938e:: with SMTP id c14mr7022561iol.88.1615480947405; Thu, 11 Mar 2021 08:42:27 -0800 (PST) MIME-Version: 1.0 References: <20210311114935.11379-1-mgorman@techsingularity.net> <20210311114935.11379-3-mgorman@techsingularity.net> In-Reply-To: <20210311114935.11379-3-mgorman@techsingularity.net> From: Alexander Duyck Date: Thu, 11 Mar 2021 08:42:16 -0800 Message-ID: Subject: Re: [PATCH 2/5] mm/page_alloc: Add a bulk page allocator To: Mel Gorman Cc: Andrew Morton , Chuck Lever , Jesper Dangaard Brouer , Christoph Hellwig , LKML , Linux-Net , Linux-MM , Linux-NFS Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: tz1bim1ibrinandyc6umguqht739st41 X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 4CC98200039F Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf18; identity=mailfrom; envelope-from=""; helo=mail-io1-f42.google.com; client-ip=209.85.166.42 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1615480949-795806 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Mar 11, 2021 at 3:49 AM Mel Gorman wrote: > > This patch adds a new page allocator interface via alloc_pages_bulk, > and __alloc_pages_bulk_nodemask. A caller requests a number of pages > to be allocated and added to a list. They can be freed in bulk using > free_pages_bulk(). > > The API is not guaranteed to return the requested number of pages and > may fail if the preferred allocation zone has limited free memory, the > cpuset changes during the allocation or page debugging decides to fail > an allocation. It's up to the caller to request more pages in batch > if necessary. > > Note that this implementation is not very efficient and could be improved > but it would require refactoring. The intent is to make it available early > to determine what semantics are required by different callers. Once the > full semantics are nailed down, it can be refactored. > > Signed-off-by: Mel Gorman > --- > include/linux/gfp.h | 13 +++++ > mm/page_alloc.c | 118 +++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 129 insertions(+), 2 deletions(-) > > diff --git a/include/linux/gfp.h b/include/linux/gfp.h > index 8572a1474e16..4903d1cc48dc 100644 > --- a/include/linux/gfp.h > +++ b/include/linux/gfp.h > @@ -515,6 +515,10 @@ static inline int arch_make_page_accessible(struct page *page) > } > #endif > > +int __alloc_pages_bulk_nodemask(gfp_t gfp_mask, int preferred_nid, > + nodemask_t *nodemask, int nr_pages, > + struct list_head *list); > + > struct page * > __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, int preferred_nid, > nodemask_t *nodemask); > @@ -525,6 +529,14 @@ __alloc_pages(gfp_t gfp_mask, unsigned int order, int preferred_nid) > return __alloc_pages_nodemask(gfp_mask, order, preferred_nid, NULL); > } > > +/* Bulk allocate order-0 pages */ > +static inline unsigned long > +alloc_pages_bulk(gfp_t gfp_mask, unsigned long nr_pages, struct list_head *list) > +{ > + return __alloc_pages_bulk_nodemask(gfp_mask, numa_mem_id(), NULL, > + nr_pages, list); > +} > + > /* > * Allocate pages, preferring the node given as nid. The node must be valid and > * online. For more general interface, see alloc_pages_node(). > @@ -594,6 +606,7 @@ void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask); > > extern void __free_pages(struct page *page, unsigned int order); > extern void free_pages(unsigned long addr, unsigned int order); > +extern void free_pages_bulk(struct list_head *list); > > struct page_frag_cache; > extern void __page_frag_cache_drain(struct page *page, unsigned int count); > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3e4b29ee2b1e..415059324dc3 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4436,6 +4436,21 @@ static void wake_all_kswapds(unsigned int order, gfp_t gfp_mask, > } > } > > +/* Drop reference counts and free order-0 pages from a list. */ > +void free_pages_bulk(struct list_head *list) > +{ > + struct page *page, *next; > + > + list_for_each_entry_safe(page, next, list, lru) { > + trace_mm_page_free_batched(page); > + if (put_page_testzero(page)) { > + list_del(&page->lru); > + __free_pages_ok(page, 0, FPI_NONE); > + } > + } > +} > +EXPORT_SYMBOL_GPL(free_pages_bulk); > + > static inline unsigned int > gfp_to_alloc_flags(gfp_t gfp_mask) > { > @@ -4919,6 +4934,9 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order, > struct alloc_context *ac, gfp_t *alloc_mask, > unsigned int *alloc_flags) > { > + gfp_mask &= gfp_allowed_mask; > + *alloc_mask = gfp_mask; > + > ac->highest_zoneidx = gfp_zone(gfp_mask); > ac->zonelist = node_zonelist(preferred_nid, gfp_mask); > ac->nodemask = nodemask; It might be better to pull this and the change from the bottom out into a seperate patch. I was reviewing this and when I hit the bottom I apparently had the same question other reviewers had wondering if it was intentional. By splitting it out it would be easier to review. > @@ -4960,6 +4978,104 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order, > return true; > } > > +/* > + * This is a batched version of the page allocator that attempts to > + * allocate nr_pages quickly from the preferred zone and add them to list. > + * > + * Returns the number of pages allocated. > + */ > +int __alloc_pages_bulk_nodemask(gfp_t gfp_mask, int preferred_nid, > + nodemask_t *nodemask, int nr_pages, > + struct list_head *alloc_list) > +{ > + struct page *page; > + unsigned long flags; > + struct zone *zone; > + struct zoneref *z; > + struct per_cpu_pages *pcp; > + struct list_head *pcp_list; > + struct alloc_context ac; > + gfp_t alloc_mask; > + unsigned int alloc_flags; > + int alloced = 0; > + > + if (nr_pages == 1) > + goto failed; I might change this to "<= 1" just to cover the case where somebody messed something up and passed a negative value. > + > + /* May set ALLOC_NOFRAGMENT, fragmentation will return 1 page. */ > + if (!prepare_alloc_pages(gfp_mask, 0, preferred_nid, nodemask, &ac, &alloc_mask, &alloc_flags)) > + return 0; > + gfp_mask = alloc_mask; > + > + /* Find an allowed local zone that meets the high watermark. */ > + for_each_zone_zonelist_nodemask(zone, z, ac.zonelist, ac.highest_zoneidx, ac.nodemask) { > + unsigned long mark; > + > + if (cpusets_enabled() && (alloc_flags & ALLOC_CPUSET) && > + !__cpuset_zone_allowed(zone, gfp_mask)) { > + continue; > + } > + > + if (nr_online_nodes > 1 && zone != ac.preferred_zoneref->zone && > + zone_to_nid(zone) != zone_to_nid(ac.preferred_zoneref->zone)) { > + goto failed; > + } > + > + mark = wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK) + nr_pages; > + if (zone_watermark_fast(zone, 0, mark, > + zonelist_zone_idx(ac.preferred_zoneref), > + alloc_flags, gfp_mask)) { > + break; > + } > + } > + if (!zone) > + return 0; > + > + /* Attempt the batch allocation */ > + local_irq_save(flags); > + pcp = &this_cpu_ptr(zone->pageset)->pcp; > + pcp_list = &pcp->lists[ac.migratetype]; > + > + while (alloced < nr_pages) { > + page = __rmqueue_pcplist(zone, ac.migratetype, alloc_flags, > + pcp, pcp_list); > + if (!page) > + break; > + > + list_add(&page->lru, alloc_list); > + alloced++; > + } > + > + if (!alloced) > + goto failed_irq; Since we already covered the case above verifying the nr_pages is greater than one it might make sense to move this check inside the loop for the !page case. Then we only are checking this if we failed an allocation. > + > + if (alloced) { Isn't this redundant? In the previous lines you already checked "alloced" was zero before jumping to the label so you shouldn't need a second check as it isn't going to change after we already verified it is non-zero. Also not a fan of the name "alloced". Maybe nr_alloc or something. Trying to make that abbreviation past tense just doesn't read right. > + __count_zid_vm_events(PGALLOC, zone_idx(zone), alloced); > + zone_statistics(zone, zone); > + } > + > + local_irq_restore(flags); > + > + /* Prep page with IRQs enabled to reduce disabled times */ > + list_for_each_entry(page, alloc_list, lru) > + prep_new_page(page, 0, gfp_mask, 0); > + > + return alloced; > + > +failed_irq: > + local_irq_restore(flags); > + > +failed: > + page = __alloc_pages_nodemask(gfp_mask, 0, preferred_nid, nodemask); > + if (page) { > + alloced++; You could be explicit here and just set alloced to 1 and make this a write instead of bothering with the increment. Either that or just simplify this and return 1 after the list_add, and return 0 in the default case assuming you didn't allocate a page. > + list_add(&page->lru, alloc_list); > + } > + > + return alloced; > +} > +EXPORT_SYMBOL_GPL(__alloc_pages_bulk_nodemask); > + > /* > * This is the 'heart' of the zoned buddy allocator. > */ > @@ -4981,8 +5097,6 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, int preferred_nid, > return NULL; > } > > - gfp_mask &= gfp_allowed_mask; > - alloc_mask = gfp_mask; > if (!prepare_alloc_pages(gfp_mask, order, preferred_nid, nodemask, &ac, &alloc_mask, &alloc_flags)) > return NULL; > > -- > 2.26.2 > >