Received: by 2002:a05:6358:d09b:b0:dc:cd0c:909e with SMTP id jc27csp1097364rwb; Thu, 1 Dec 2022 12:19:39 -0800 (PST) X-Google-Smtp-Source: AA0mqf7kZ4aaLD5FgmQF4qV0WTzU2cFZ39wNY7YJMPVhHrpQ5CTHAbc7ANexWvyQAPaACH2xVuP6 X-Received: by 2002:a17:906:d20a:b0:7a9:8d8e:c3df with SMTP id w10-20020a170906d20a00b007a98d8ec3dfmr41881518ejz.519.1669925979141; Thu, 01 Dec 2022 12:19:39 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669925979; cv=none; d=google.com; s=arc-20160816; b=jjIgvKkwIuU6i0QbDW+4fjej1Uc6Bz6YOARAgrV777OkloZdYD7NyWbhmVPLAsZbU9 OTlnmT4D9XJB9tu+RkEZPRn7GsMHsFye2fLRvDiFvLOljO6XLKzrGN3x6dE7Uw3fVREV dPv+xPV6YfS/8MgoUuZFm6CQ7/sLeDWvDGs2QNKMv6juEkOUcTlWEGrBMCqUeBezrp8Y Jpus9gYrMMObDukIwWyZbxXIFfHUH+OaZ9WZBUD1m9xd1RZAswD6Dd1KKDPDQCZnn8ab zB8H4006C5nuJfNL72/AOhQkcV8/OhwZDWdiDcoHHrSzlM9l2SBrBCfis6mc3+7ehdHU WcCQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=Nbpk4AkvtKLAPHTKixeSgzLmRJlY8WI7hSqD7VhAlxQ=; b=cFJ7jf/pG6ajRSZut3/8bdrWZ30zc05NX9CwEloVl8WyADS6cJFkcp8nMaFZxFVnHW 7lBjwp4/Uw5P1S0NiPsHXUxdN2NgiS+GSkf6QJgEmNAzBnJjiKVlCMtGh8uLuFBG9kGB mOk9GgYkOnlbq0lqgH+7aTeDEmEQM+ZVXvwjVxFLuNqlROR3oXf7jrOTbHMaP4nFLfVO FDk6a08n9DRM3Dgdduyhu47eoQAl/CLdP6ch9ojkX1ZV5S+Dyi2cof7lItiZgCj75rL0 Tefwzuuvcc+aJDDbXDgcxf+qQAi2bdtKo0D4V7133EsMswRl5QwBMUaR90nMyV0u7Pdb hdpA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=mAR73VxM; 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=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id r23-20020a056402035700b00458d43beac7si3812271edw.41.2022.12.01.12.19.19; Thu, 01 Dec 2022 12:19:39 -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=@google.com header.s=20210112 header.b=mAR73VxM; 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=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230331AbiLATsM (ORCPT + 82 others); Thu, 1 Dec 2022 14:48:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46106 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230022AbiLATsL (ORCPT ); Thu, 1 Dec 2022 14:48:11 -0500 Received: from mail-ua1-x92a.google.com (mail-ua1-x92a.google.com [IPv6:2607:f8b0:4864:20::92a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5D75E5444A for ; Thu, 1 Dec 2022 11:48:10 -0800 (PST) Received: by mail-ua1-x92a.google.com with SMTP id e26so965011uaa.7 for ; Thu, 01 Dec 2022 11:48:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Nbpk4AkvtKLAPHTKixeSgzLmRJlY8WI7hSqD7VhAlxQ=; b=mAR73VxMIhpsSpn+ZYfJYZpFuC/OQBiKV6Hv7ryKTrq7ru56rD6I2rD1d1c0DozVHj oeWBiWTle2FTv/tbzT3J1U87aQwBBkEKKH17ISZWygb/CEGMs98Oif75QgNRCVa2vzim dlJdNW7RBki0Jf6XTvbJwUupKT/LaAHT04sXmEw9iapUtpLUFaIIp6uuxIJ4ksYhitnQ PL5HIv00SQefRQqTqc+PSNFLE3Fu/asJnDvr0SkmH5FpQt8JS8l3dQvC0TSewMUyoFb5 TXdNWq1QpeVgDJ5JTB2oY7VFDeZOLKbdQt7KgsvnC4AMM33xgBda6uWwmnMAL+h/BbYk +NMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Nbpk4AkvtKLAPHTKixeSgzLmRJlY8WI7hSqD7VhAlxQ=; b=ilFukmJMxONDdyh1VklJPqdsanxuZ7MJV3i3ptQaMTAtXeE2LRjGMog1xv27BIJYA+ M5KD/Ax3fwvqfci+r9HG9qSrPBNYQZ7bixPChU77AItNdZfM1bgyAw+mtgT33am+oK+o MQJwKJTyDJ+mypuhi0TJ0xBduLknGM6CLVTDWp3D6+Qgt3qP4eeY/PkkzdFkAaJ4JvPE nXQ+NdxtZmkca2QlxK04ranneuW3UT3IXN4Vm5WtWRVKAm5QS+JHkYFTa9urDJG9F588 tMmfikFs6coJRfJjFp9vT9ynBeR6n0cha8sBYZW09LdHEu7PnSdd26isop4aewX9YS0y uwQQ== X-Gm-Message-State: ANoB5pl/9ypx5Ysx6Z+y5IbipZBN7P5KnV+yA+e0xinzUQgDbQPSLTbw oa6cZ03hlqO+ueVqR4pikrqmgSynjsUIM6uTR+DXow== X-Received: by 2002:ab0:1602:0:b0:3c7:9fbd:a455 with SMTP id k2-20020ab01602000000b003c79fbda455mr32052467uae.113.1669924089303; Thu, 01 Dec 2022 11:48:09 -0800 (PST) MIME-Version: 1.0 References: <20221201192644.1941049-1-axelrasmussen@google.com> In-Reply-To: <20221201192644.1941049-1-axelrasmussen@google.com> From: Yu Zhao Date: Thu, 1 Dec 2022 12:47:33 -0700 Message-ID: Subject: Re: [PATCH] mm: multi-gen LRU: fix LRU size accounting on folio removal To: Axel Rasmussen Cc: Andrew Morton , Suleiman Souhlal , Steven Barrett , Oleksandr Natalenko , Suren Baghdasaryan , Arnd Bergmann , Peter Xu , Gaosheng Cui , Hugh Dickins , Brian Geffon , "Jan Alexander Steffens (heftig)" , linux-kernel@vger.kernel.org, Yosry Ahmed Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-17.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL 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: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 1, 2022 at 12:26 PM Axel Rasmussen wrote: > > When removing a folio from MGLRU, we want to update the LRU size > accordingly based on the generation it belonged to previously - > lru_gen_update_size() does this. > > The bug here is, set_mask_bits effectively clears the generation bits. > Ignoring the complexity set_mask_bits is meant to handle, the code being > changed here is in effect: > > flags = !reclaiming && lru_gen_is_active(lruvec, gen) ? BIT(PG_active) : 0; > flags = *folio->flags = (*folio->flags & ~LRU_GEN_MASK) | flags; > gen = ((flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1; > > In other words, the bug is we clear all of the `LGU_GEN_MASK` bits, and > then we recalculate `gen` - but of course after clearing the bits > `flags & LRU_GEN_MASK` is always zero, and so now `gen` is always -1. > > So we effectively always call: > > lru_gen_update_size(lruvec, folio, -1, -1); > > This leads `lru_gen_update_size` to incorrectly conclude that we're > **adding**, not removing, a folio. We take this path: > > /* addition */ > if (old_gen < 0) { > /* always false, new_gen is -1 too */ > if (lru_gen_is_active(lruvec, new_gen)) > /* ... */ > __update_lru_size(lruvec, lru, zone, delta); > return; > } > > In other words, when removing, we incorrectly *add* the delta to the > inactive LRU instead of subtracting. > > The fix is simple. We already have the generation number the folio > belonged to: we set `int gen = folio_lru_gen(folio);` at the top of > `lru_gen_del_folio`. So, just delete the line incorrectly recalculating > the generation number. > > Fixes: ec1c86b25f4b ("mm: multi-gen LRU: groundwork") > Signed-off-by: Axel Rasmussen NAK. You are referencing our old (9xx) set_mask_bits(), which returns "new" (a bad behavior). Its latest version returns "old". Even if it was a bug: 1. lru_gen_update_size(lruvec, folio, -1, -1) would have been caught by VM_WARN_ON_ONCE(old_gen == -1 && new_gen == -1). 2. The fix is still wrong, because "gen" read from folio_lru_gen(folio) is non atomic and can change before set_mask_bits() finishes.