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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABE91CCA47A for ; Thu, 16 Jun 2022 21:08:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2B51C6B0082; Thu, 16 Jun 2022 17:08:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 263BD6B0083; Thu, 16 Jun 2022 17:08:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 15F0C6B0085; Thu, 16 Jun 2022 17:08:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 06E6E6B0082 for ; Thu, 16 Jun 2022 17:08:03 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D17CA60790 for ; Thu, 16 Jun 2022 21:08:02 +0000 (UTC) X-FDA: 79585336404.10.00BF7EE Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) by imf14.hostedemail.com (Postfix) with ESMTP id 6E74F100072 for ; Thu, 16 Jun 2022 21:08:02 +0000 (UTC) Received: by mail-vs1-f50.google.com with SMTP id x187so2381100vsb.0 for ; Thu, 16 Jun 2022 14:08:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QYZC9iLsYDsaByy8JmaeJ2G98wNd/D+AjNOCL1AQkNs=; b=F6ti4Z21S49N0+AFdXj43ViP0q+ENG4foOKMonRNclvKZBW2Lz71cWLsGLb3e0pxmh c7zxQp9KWZHNh2Z1LHpAYqy64o69U1exvhyBf179TaHvNv6tq6qJ025FmKxQFzD4jcJx QhTSdSd4ThqXTzQZW8iLRFyIiSvv05+u2ihxE9uerdH8SaVYCuqk3DwSfA9RNFJl8Cn5 DLEKdPcR1QoIPoldZX9tsbV2NDOJlMnXHMAfLXR0/9AVab9W2U4acuCBFVLwGR4zVuqN 2ZOBN5vi/s88FNqX9UHmMxL3XBdFHS3CRJZ8kAE1bEaFZQbfwIyj3biE0daSpRINUZaq KTWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QYZC9iLsYDsaByy8JmaeJ2G98wNd/D+AjNOCL1AQkNs=; b=pbhAQ60m7CtqfxmGT3OxtfHF85gSV8LysYkTD16kjtXfPUYas47pqw4RLlcK7QkMG+ cZlQtAqchCtgvYdkWH1/aJ8HmyTyxBxHuHLREnjr7TfSjQjNl40iQmiOuFXsQ2fo5Iu3 K7lsAWu9+q8+eeIiqhQ/3Cm65gqe2B4wNEPCsi+WXW/A0hVaf9uTari1rOXQolYGsYOS ZBYGURqNPB1FAI54ezhc8rYD7E5MHELSZxGuNRBRQM4I8DQCOB2Y7qZMI2br+SaxyosI BVfE7jwoVzHEfkDbW8IZPaZJEkrFWfWjEzeXo9FZCnHiN2Dim6kzSwpxicZDiH5n7owU 7V7A== X-Gm-Message-State: AJIora/VyAGAFNyX1ZpFGrytZT7ps/FxWtPdc3JeV1+QTBzG+GLuCAOY Ef5HlYBlJdseVjXYUQnK006z1PRaE+7GmzSuDdAV4Q== X-Google-Smtp-Source: AGRyM1uFolbScaQC6FfwKtMgJbBm1M0CZmgiZxWFQphbt2Z6rX7ZCeXMZ+tOLGbXinOqLYqUo+b1hhp8iEXBMc37I8s= X-Received: by 2002:a05:6102:3e23:b0:34b:b6b0:2ae7 with SMTP id j35-20020a0561023e2300b0034bb6b02ae7mr3534210vsv.81.1655413681460; Thu, 16 Jun 2022 14:08:01 -0700 (PDT) MIME-Version: 1.0 References: <20220613125622.18628-1-mgorman@techsingularity.net> <20220613125622.18628-8-mgorman@techsingularity.net> <43033655-2e78-621b-cc76-c3dc53024d00@suse.cz> In-Reply-To: <43033655-2e78-621b-cc76-c3dc53024d00@suse.cz> From: Yu Zhao Date: Thu, 16 Jun 2022 15:07:25 -0600 Message-ID: Subject: Re: [PATCH 7/7] mm/page_alloc: Replace local_lock with normal spinlock To: Vlastimil Babka Cc: Mel Gorman , Andrew Morton , Nicolas Saenz Julienne , Marcelo Tosatti , Michal Hocko , Hugh Dickins , LKML , Linux-MM Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655413682; a=rsa-sha256; cv=none; b=jOos+AUS+tJJVCofyJQD9/Le8HnsYGxl8QxI1XTvTfvk48q0napD8WEJrhKxaaTx4MECjf 72YwprEPDzDPjAVp381uqR052JNKC1gCAvXod83gFpCkJfm8tD0LSPavoIO306jJ8cYpq2 XiwFJ20YHf9dXL9u0HtPkCJGvxENqDs= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=F6ti4Z21; spf=pass (imf14.hostedemail.com: domain of yuzhao@google.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655413682; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=QYZC9iLsYDsaByy8JmaeJ2G98wNd/D+AjNOCL1AQkNs=; b=V7OUTmvykzjG4KBy3QrhtUfOsaowZ4aeAB5PghrbkzYXFRLg4g6Y1FeMGEEWGpLjpm+GQj DA05Gatw2fI9gV20PfbSR6JJP2LsA7oEfadhEIbCaIupyPZiM6/ZyFsoUmishjodNz9RMi Hma4eP0YBEhV24kA31vek1Q57i1WJl0= X-Stat-Signature: 8hqxhfk8ozg3wuzq5e1mqbtyqepxz1e1 X-Rspamd-Queue-Id: 6E74F100072 Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=F6ti4Z21; spf=pass (imf14.hostedemail.com: domain of yuzhao@google.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam07 X-Rspam-User: X-HE-Tag: 1655413682-396501 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, Jun 16, 2022 at 11:02 AM Vlastimil Babka wrote: > > On 6/13/22 14:56, Mel Gorman wrote: > > struct per_cpu_pages is no longer strictly local as PCP lists can be > > drained remotely using a lock for protection. While the use of local_lock > > works, it goes against the intent of local_lock which is for "pure > > CPU local concurrency control mechanisms and not suited for inter-CPU > > concurrency control" (Documentation/locking/locktypes.rst) > > > > local_lock protects against migration between when the percpu pointer is > > accessed and the pcp->lock acquired. The lock acquisition is a preemption > > point so in the worst case, a task could migrate to another NUMA node > > and accidentally allocate remote memory. The main requirement is to pin > > the task to a CPU that is suitable for PREEMPT_RT and !PREEMPT_RT. > > > > Replace local_lock with helpers that pin a task to a CPU, lookup the > > per-cpu structure and acquire the embedded lock. It's similar to local_lock > > without breaking the intent behind the API. It is not a complete API > > as only the parts needed for PCP-alloc are implemented but in theory, > > the generic helpers could be promoted to a general API if there was > > demand for an embedded lock within a per-cpu struct with a guarantee > > that the per-cpu structure locked matches the running CPU and cannot use > > get_cpu_var due to RT concerns. PCP requires these semantics to avoid > > accidentally allocating remote memory. > > > > Signed-off-by: Mel Gorman > > ... > > > @@ -3367,30 +3429,17 @@ static int nr_pcp_high(struct per_cpu_pages *pcp, struct zone *zone, > > return min(READ_ONCE(pcp->batch) << 2, high); > > } > > > > -/* Returns true if the page was committed to the per-cpu list. */ > > -static bool free_unref_page_commit(struct page *page, int migratetype, > > - unsigned int order, bool locked) > > +static void free_unref_page_commit(struct per_cpu_pages *pcp, struct zone *zone, > > + struct page *page, int migratetype, > > + unsigned int order) > > Hmm given this drops the "bool locked" and bool return value again, my > suggestion for patch 5/7 would result in less churn as those woudn't need to > be introduced? > > ... > > > @@ -3794,19 +3805,29 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > struct list_head *list; > > struct page *page; > > unsigned long flags; > > + unsigned long __maybe_unused UP_flags; > > > > - local_lock_irqsave(&pagesets.lock, flags); > > + /* > > + * spin_trylock_irqsave is not necessary right now as it'll only be > > + * true when contending with a remote drain. It's in place as a > > + * preparation step before converting pcp locking to spin_trylock > > + * to protect against IRQ reentry. > > + */ > > + pcp_trylock_prepare(UP_flags); > > + pcp = pcp_spin_trylock_irqsave(zone->per_cpu_pageset, flags); > > + if (!pcp) > > Besides the missing unpin Andrew fixed, I think also this is missing > pcp_trylock_finish(UP_flags); ? spin_trylock only fails when trylock_finish is a NOP.