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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 50D1BC47083 for ; Mon, 24 May 2021 09:12:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 23769611EE for ; Mon, 24 May 2021 09:12:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232487AbhEXJNw (ORCPT ); Mon, 24 May 2021 05:13:52 -0400 Received: from outbound-smtp25.blacknight.com ([81.17.249.193]:38439 "EHLO outbound-smtp25.blacknight.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232313AbhEXJNv (ORCPT ); Mon, 24 May 2021 05:13:51 -0400 Received: from mail.blacknight.com (pemlinmail05.blacknight.ie [81.17.254.26]) by outbound-smtp25.blacknight.com (Postfix) with ESMTPS id A0FC4CAD42 for ; Mon, 24 May 2021 10:12:22 +0100 (IST) Received: (qmail 26294 invoked from network); 24 May 2021 09:12:22 -0000 Received: from unknown (HELO techsingularity.net) (mgorman@techsingularity.net@[84.203.23.168]) by 81.17.254.9 with ESMTPSA (AES256-SHA encrypted, authenticated); 24 May 2021 09:12:22 -0000 Date: Mon, 24 May 2021 10:12:20 +0100 From: Mel Gorman To: Dave Hansen Cc: Linux-MM , Dave Hansen , Matthew Wilcox , Vlastimil Babka , Michal Hocko , Nicholas Piggin , LKML Subject: Re: [PATCH 4/6] mm/page_alloc: Scale the number of pages that are batch freed Message-ID: <20210524091220.GC30378@techsingularity.net> References: <20210521102826.28552-1-mgorman@techsingularity.net> <20210521102826.28552-5-mgorman@techsingularity.net> <8646d3ad-345f-7ec7-fe4a-ada2680487a3@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <8646d3ad-345f-7ec7-fe4a-ada2680487a3@intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 21, 2021 at 03:36:05PM -0700, Dave Hansen wrote: > ... > > +static int nr_pcp_free(struct per_cpu_pages *pcp, int high, int batch) > > +{ > > + int min_nr_free, max_nr_free; > > + > > + /* Check for PCP disabled or boot pageset */ > > + if (unlikely(high < batch)) > > + return 1; > > + > > + min_nr_free = batch; > > + max_nr_free = high - batch; > > I puzzled over this for a minute. I *think* it means to say: "Leave at > least one batch worth of pages in the pcp at all times so that the next > allocation can still be satisfied from this pcp." > Yes, I added a comment. > > + batch <<= pcp->free_factor; > > + if (batch < max_nr_free) > > + pcp->free_factor++; > > + batch = clamp(batch, min_nr_free, max_nr_free); > > + > > + return batch; > > +} > > + > > static void free_unref_page_commit(struct page *page, unsigned long pfn, > > int migratetype) > > { > > struct zone *zone = page_zone(page); > > struct per_cpu_pages *pcp; > > + int high; > > > > __count_vm_event(PGFREE); > > pcp = this_cpu_ptr(zone->per_cpu_pageset); > > list_add(&page->lru, &pcp->lists[migratetype]); > > pcp->count++; > > - if (pcp->count >= READ_ONCE(pcp->high)) > > - free_pcppages_bulk(zone, READ_ONCE(pcp->batch), pcp); > > + high = READ_ONCE(pcp->high); > > + if (pcp->count >= high) { > > + int batch = READ_ONCE(pcp->batch); > > + > > + free_pcppages_bulk(zone, nr_pcp_free(pcp, high, batch), pcp); > > + } > > } > > > > /* > > @@ -3531,6 +3555,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > > local_lock_irqsave(&pagesets.lock, flags); > > pcp = this_cpu_ptr(zone->per_cpu_pageset); > > + pcp->free_factor >>= 1; > > list = &pcp->lists[migratetype]; > > page = __rmqueue_pcplist(zone, migratetype, alloc_flags, pcp, list); > > local_unlock_irqrestore(&pagesets.lock, flags); > > A high-level description of the algorithm in the changelog would also be > nice. I *think* it's basically: > > After hitting the high pcp mark, free one pcp->batch at a time. But, as > subsequent pcp free operations occur, keep doubling the size of the > freed batches. Cap them so that they always leave at least one > pcp->batch worth of pages. Scale the size back down by half whenever an > allocation that consumes a page from the pcp occurs. > > While I'd appreciate another comment or two, I do think this is worth > doing, and the approach seems sound: > > Acked-by: Dave Hansen Thanks, I added a few additional comments. -- Mel Gorman SUSE Labs