From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 65A0522D4DC for ; Thu, 16 Jan 2025 18:46:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737053174; cv=none; b=eca1yQ6elP6Q2qYuANHwDxWuNxGEL9iopN5sfn8nr3trnBRmvOpiVwNm7j3Io3BjSesfsHCme4My232ZPAdnTGa+xgn4mS2MRB1r5cECjVDtDEGPnF29d3jKsqvxS4eHuCfQZVlUqemwYGFlrfb8eQE+9zC7hk+gW95C4cdSOxI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737053174; c=relaxed/simple; bh=WF3r+rAK8abcTs798j0cGBNqtejnxRiE5TaTiGRYFUk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Ai60WS4ZGQe5LcjG1hLRukIsChEvM8/89GNNsWSgnOJfFbLZdgyFfdy4J6QreT/q5O22a9qOjcOEknRnoKgQ19TpPor8lWjFZ7jfj4vVCZUHqjYgcZ15HPJuhbWbxYeCfg6uBt1exUJwMlmUE1jQjCF8LSpalp3p+PLsYkGP5TU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=d5GRavAj; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="d5GRavAj" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=p9O/iRU1RGhq+Ms0ySqk0/+MMJp9XVeXHuplWuBrmf0=; b=d5GRavAjou3IC+5i1tBHyZi1jE I+9+P4XG0BPYLwLHii5QW9FbXTIRJQjWtHC8OgtAofvP6lvZ5GH1gl0TS1ZNjfHiEEpMMSwhMPAj9 3AGiRdUTt6vF81HRIZG85lDTrmtoSfEnlvCgrE5mD9tgiCiIlbJZtU5TJPcMc4veE64T7/+irC/SZ xNjjLL32FC+T4TkIsn7VUODa4IPdNbRivoF/fbx6HSX32iRMERiCUq3DP+2ZViBm57p+DEzusynQ+ r2u6a0oL7iJGlMQ/kEN0WH8ls2kgZS3UFtyoKEupD8MaFwVEt4YusKVmJVQ30bu000z/W70jgy8ZO bCGfV39A==; Received: from willy by casper.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1tYUsY-00000000pIn-2LUg; Thu, 16 Jan 2025 18:46:06 +0000 Date: Thu, 16 Jan 2025 18:46:06 +0000 From: Matthew Wilcox To: Donet Tom Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Gregory Price , Yu Zhao , Ritesh Harjani , "Aneesh Kumar K . V" , David Hildenbrand , Huang Ying , Johannes Weiner Subject: Re: [RFC] mm/swap.c: Enable promotion of unmapped MGLRU page cache pages Message-ID: References: <20250115120625.3785-1-donettom@linux.ibm.com> <7f370dd2-a1d2-4b35-ac40-0380633846fc@linux.ibm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7f370dd2-a1d2-4b35-ac40-0380633846fc@linux.ibm.com> On Thu, Jan 16, 2025 at 04:40:21PM +0530, Donet Tom wrote: > > Should there be an 'else' clause here? Or should it be: > > > > lruvec = mem_cgroup_lruvec(memcg, folio_pgdat(folio)); > > gen = folio_lru_gen(folio); > > if ((gen < MAX_NR_GENS) && lru_gen_is_active(lruvec, gen)) > > promotion_candidate(folio); > > > Yes, there should be an else block to handle the promotion of non memcg > folios. > > Does this approach look correct to you? No. Look at the implementation of folio_memcg(), and mem_cgroup_lruvec() for that matter. >         } while (!try_cmpxchg(&folio->flags, &old_flags, new_flags)); > + > +promo_candid: > +       if (!folio_test_isolated(folio) && > +               (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) > && > +               numa_pagecache_promotion_enabled) { > +               pgdat = folio_pgdat(folio); > +               gen = folio_lru_gen(folio); > + > +               if (mem_cgroup_disabled()) > +                       lruvec = &pgdat->__lruvec; > +               else { > +                       memcg = folio_memcg(folio); > +                       lruvec = mem_cgroup_lruvec(memcg, pgdat); > +               } > + > +               if ((gen < MAX_NR_GENS) && lru_gen_is_active(lruvec, gen)) > +                       promotion_candidate(folio); > +       } >  } > >