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 8AF44C25B4E for ; Fri, 20 Jan 2023 18:52:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 052B16B0071; Fri, 20 Jan 2023 13:52:40 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 002056B0073; Fri, 20 Jan 2023 13:52:39 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E0C3D6B0074; Fri, 20 Jan 2023 13:52:39 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id D2A936B0071 for ; Fri, 20 Jan 2023 13:52:39 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id A85601409FD for ; Fri, 20 Jan 2023 18:52:39 +0000 (UTC) X-FDA: 80376073638.25.7F025B0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf23.hostedemail.com (Postfix) with ESMTP id 2E787140010 for ; Fri, 20 Jan 2023 18:52:36 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=NsxQIerP; spf=pass (imf23.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1674240757; 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=LLA0P2Oo0NSyC29FxsEUtLQkfAdXpwLv+bHSoAALBfc=; b=IvdKPHGf9oTd0LlG27HvqEyJONdbGcFVCFgSZNAV9Z20r8D1G7NOjDLVgC4jFiesJ/KKWt PiLCP4T9m1Dc4WwNgaZPplYe4rg3ol6WpNxV9+bOlWJCbw6MdFFaNssxUOEgf96QkY+WwR q95yODkZwkRnQ7JHOM7BeL/dVYdCWJI= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=NsxQIerP; spf=pass (imf23.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1674240757; a=rsa-sha256; cv=none; b=RYFv5lzNo0LTME1mVceQ/o61iCneezyMpYhLe/iTTfUYHaQhBGbp2lOSrrDLyM+FwBEWz7 5GYH1dd2E2Iae7eITgm+MsEkDcEd88BV+lT3/o7BW+EoNcPtIVhkJiXyqS6CVomO80pRy+ o35cVWE5RJ70db+b69I/n+Ea7UxTrK8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674240756; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=LLA0P2Oo0NSyC29FxsEUtLQkfAdXpwLv+bHSoAALBfc=; b=NsxQIerPoAxO4tTnJVEdHv8B1GNgwZxd61CI3zbQUlE+0xQ+LmliRD5x2BVd8xbMaKpJiQ c14HNFBpzSRhkqPNjwLEYq3j0Jua/n6HVSeBgkTyu6AmkynXHpunFen0GGR9GXWEy+MW6f r0mvxOjH8npBCG0jc5C4+AWT15uc+r4= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-150-rtisxj2CPVifIwaT-qfgxA-1; Fri, 20 Jan 2023 13:52:35 -0500 X-MC-Unique: rtisxj2CPVifIwaT-qfgxA-1 Received: by mail-qv1-f70.google.com with SMTP id x6-20020a0cc506000000b005349c8b39d6so2867068qvi.2 for ; Fri, 20 Jan 2023 10:52:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=LLA0P2Oo0NSyC29FxsEUtLQkfAdXpwLv+bHSoAALBfc=; b=2WUVQkgJoVurbDMLUlV6zSOWCGsTVx5QzdULymDytlV1w5aojBTnCUe4LwcBucAaoV 1jJvqVpbjwuDSbiMH+L3QkF/8wPMN7/9eXeoN+iucRqGvLsjRM9TdsnowuMuM6+ka+Os 47V8ilGHXXSrNWYxy6sPH9bzAvNxzbCOYtUwWLz6LAhmmL3Br1pNj6mkeDvkOvdoxgUc cO3IOoYrKezunyi/LCOlyzAbvDqf7n3vRHiF7ZIR/8OY6TV5ID2Z6LQxHzd996G0JDce 4Az1C6BiwGtpaHCD5BlUDvTcQkSI2+TqcajmHrwxlCl086HCaz1zJscIcmMVYmbGbJsw Wb/A== X-Gm-Message-State: AFqh2kp10z+HtN6eLES+yahZ54WEUDhcV12Q+YQhOOGe3AdF1X2JawJl VcNNI7LAukKswVhrkUUvfJS+Rt/3oSY3hXvtugaCr/WZqqerjlvQQm+wpgKnUdho5Ya9gQzmTaf qpZiFUFY2U6o= X-Received: by 2002:a0c:e7ca:0:b0:537:4b24:7fba with SMTP id c10-20020a0ce7ca000000b005374b247fbamr2704053qvo.28.1674240754715; Fri, 20 Jan 2023 10:52:34 -0800 (PST) X-Google-Smtp-Source: AMrXdXt/+92+YuFUWrLFI6DSkbz2bjUdyl+N/U/fZqMKdgKCs/hwOTOdimPPjygjVc9xJzylT3w3qg== X-Received: by 2002:a0c:e7ca:0:b0:537:4b24:7fba with SMTP id c10-20020a0ce7ca000000b005374b247fbamr2704035qvo.28.1674240754479; Fri, 20 Jan 2023 10:52:34 -0800 (PST) Received: from bfoster (c-24-61-119-116.hsd1.ma.comcast.net. [24.61.119.116]) by smtp.gmail.com with ESMTPSA id m4-20020a05620a290400b007069fde14a6sm7549789qkp.25.2023.01.20.10.52.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Jan 2023 10:52:33 -0800 (PST) Date: Fri, 20 Jan 2023 13:53:36 -0500 From: Brian Foster To: Johannes Weiner Cc: Nhat Pham , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, willy@infradead.org, linux-api@vger.kernel.org, kernel-team@meta.com, Yu Zhao Subject: Re: [PATCH v6 1/3] workingset: refactor LRU refault to expose refault recency check Message-ID: References: <20230117195959.29768-1-nphamcs@gmail.com> <20230117195959.29768-2-nphamcs@gmail.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 2E787140010 X-Rspam-User: X-Stat-Signature: 46oqeze6jgn8q3pyn5gjfh4w45r13og6 X-HE-Tag: 1674240756-641524 X-HE-Meta: U2FsdGVkX19/GxOvMMgZImuCnqfGJlm1FGgIXi6dxDWpe3i5j9hibTgC+vDIIrxJ954wsmEne2Jng20MwhvLkDy80xqTlpCqfhppIHjQkydLJwmSGRtsfkK2T3RqSBX20IAJdUL78fHgbSS67qRW7IFYP0GGLU5Ar7dGtV4X5uQCzjBJeoHcpnJdE/u16QZ2RjQPVMt9DRjVM+Nojih2XakOXiPQMcpecTmoh5HLhGpKJzSAL8pb0zV+y4uYuGZ4gk9uGFHpwKA0RWfSMZnKAUqjg0q10/9oDoopPLaaHL585ZTdV46byWDX2kRbp9y5g0APfYMu/9u2iFH/h+usy5GB/vy4xu7xc91DbEGlo8ek/oc+BHvzIkuTI2amlp9WbA2djr/2z+kdvQgkSHgv/ryz+flc8FSswDUEsa2ORTkd1oiWgCqPaTEXw1MIsPbHb3LNYC3pBcTT/68KxEDuFWqc76zcfX7dfgyNWCvh0VOM9SuAUo0Tz6zQ9p2g9sfW+4yhjdGHdvOuGE2v+EpprgE5w8LrUDEMIGgEl22TfJyZjM7bt6sYn9xFWm0IOWDUrB5ihwJYGkpsUJNNzfjomZPbcjAfJ7bbbrMnWVK38hUOnu3xoRCZlhFCP00iBsSG1odgj4AN78QnWUA64JtHSb3viQrgHmbEaSmW0wxuJAoxc0EyENCAaoXgEWtTAW69g06lDi+ZyMegFWWpdSwpvQFJb7e0qH8+Bnm2GT/JgzchCAfoBeH7hkWLRIA49qRR3Dfl7cGJG+ctDrqu5xBhmdpUVeLWUdG53ZBl6FH6KRENhOTifIxebcp6PSsORIeH/EB2BuqMPF2+VyEDUWOca+p2M7suj5UxHajCsRuF3oj/wRlUN4hsJB1Gqy6jkjVDH4V3LgjrMijqTfH5snJbh55m32nA89Tlbq0kxmp9Y0SWldJ+BL5Wd6k7CZZRnuJXct+2EKkRiIm44mKQIt7 uDZ50vIX pd1OMIuRENlxW1oM4y2n8VTVrXxjbiJILt5cjhkEOTaMYB5A2EJJP2XAv64/HWMIKfhyw0RKGtlwIX+lo98DpsRKhjSmr1olUvDG69R4CmY8JWWmFamIOKdvvQGqm4eYyBt6SXRhgLjLL53PMFQ1lgijXKX1nciPOYlEzgrINBbm4NDHl646nEQsZRUzBjLUDfaopAMipQPcL1tA/UGomtgWeGqfE6xZPlUHS9GQ2iA96o4RO1zZYVmDfsqdKykoqQslxnU6Al0gKwfGJ3wJAP+lFgicjt06ypE5nEtNJxUTpaegEPevWj4vi2lS45gJsUTjQpOUInCitqS91xcdNA/HV2+8LfuxjY7lVl4FDmZ6CefYIidXEqnUYJB46DEJDUHxmJEew6YQlR6CIonhDPzWOR3wLIWYIkfB0qdat1uiKph3PAJG/3TvA57EtcQ4SozFUxSQblhjXp88= 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 Fri, Jan 20, 2023 at 11:27:12AM -0500, Johannes Weiner wrote: > On Fri, Jan 20, 2023 at 09:34:18AM -0500, Brian Foster wrote: > > On Tue, Jan 17, 2023 at 11:59:57AM -0800, Nhat Pham wrote: > > > + int memcgid; > > > + struct pglist_data *pgdat; > > > + unsigned long token; > > > + > > > + unpack_shadow(shadow, &memcgid, &pgdat, &token, workingset); > > > + eviction_memcg = mem_cgroup_from_id(memcgid); > > > + > > > + lruvec = mem_cgroup_lruvec(eviction_memcg, pgdat); > > > + lrugen = &lruvec->lrugen; > > > + > > > + min_seq = READ_ONCE(lrugen->min_seq[file]); > > > + return !((token >> LRU_REFS_WIDTH) != (min_seq & (EVICTION_MASK >> LRU_REFS_WIDTH))); > > > > I think this might be more readable without the double negative. > > > > Also it looks like this logic is pulled from lru_gen_refault(). Any > > reason the caller isn't refactored to use this helper, similar to how > > workingset_refault() is modified? It seems like a potential landmine to > > duplicate the logic here for cachestat purposes and somewhere else for > > actual workingset management. > > The initial version was refactored. Yu explicitly requested it be > duplicated [1] to cut down on some boiler plate. > Ah, sorry for missing the previous discussion. TBH I wasn't terribly comfortable reviewing this one until I had made enough passes at the second patch.. > I have to agree with Brian on this one, though. The factored version > is better for maintenance than duplicating the core logic here. Even > if it ends up a bit more boiler plate - it's harder to screw that up, > and easier to catch at compile time, than the duplicates diverging. > It seems more elegant to me, FWIW. Glad I'm not totally off the rails at least. ;) But I'll defer to those who know the code better and the author, so that's just my .02. I don't want to cause this to go around in circles.. Brian > [1] https://lore.kernel.org/lkml/CAOUHufZKTqoD2rFwrX9-eCknBmeWqP88rZ7X7A_5KHHbGBUP=A@mail.gmail.com/ >