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 1019FC54E58 for ; Thu, 21 Mar 2024 16:04:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 66E616B007B; Thu, 21 Mar 2024 12:04:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 61F496B0082; Thu, 21 Mar 2024 12:04:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4E7DE6B0083; Thu, 21 Mar 2024 12:04:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 3DE3A6B007B for ; Thu, 21 Mar 2024 12:04:04 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 86CCFC0B30 for ; Thu, 21 Mar 2024 16:04:02 +0000 (UTC) X-FDA: 81921517524.21.36B846B Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf26.hostedemail.com (Postfix) with ESMTP id 6D6E2140046 for ; Thu, 21 Mar 2024 16:03:56 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=fzSn3kGo; spf=none (imf26.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711037039; a=rsa-sha256; cv=none; b=nG590JSphPvGzeG+HpN7mkgCoqD/OLy+Fv06D3v9van345th/pTP2LLSJaNEuWpnGaT1dO fq2N7ckanZOjJk4MK1jPIyGdvXNg2ByHPyHe5mPP1h1yoQ2h/GG6q9YIK1HkEor4BBEdVC tBhfmYu3K4ltWJRmVLYbe0Jc1XveCYY= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=fzSn3kGo; spf=none (imf26.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711037039; 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=X4zP7bOt9aFfiZj3Kbm1qwQHpwNl/4ORsgZCeBn0NbY=; b=pWmKNG3gDOkQ3Wcv8WjaVzRWoHtAVOq36J2O4WSxSchg1um7jj5m2y+s8x/QZ6CLRrLeaf aUjAFgzOniPv6EwjCf60UecvdNVAjmly6rScuSplVhTSX+Clo9iENRwzjq+qcwAjDQLQ5E eMDxJnOS1qmbunG5puzdTVtMi7U6mfk= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=X4zP7bOt9aFfiZj3Kbm1qwQHpwNl/4ORsgZCeBn0NbY=; b=fzSn3kGo+M5viTVcPwz//RbdMM 4NIff+NJ3wzRKLeQBW/CNM40FAMVXu+el/afBBpYvWTqfUhsyliRw1LjB9Mmy1VX/Opc4TPLlvxf+ mcWxwPUlsrBkDl9QQxq6N//YhUpk8Oj8HMl8N6OLbgtMCwf1HbOiDz6yTl+VukDjvSTQUbXjye2Df ISjkTpAr1tzeGN7U7dTK4WO6xCYTsq8HGHpRsyWk5CuQUR5i+i4SiK7yAj+ZNIjv1Fw1TUKl5ELTm vAxWosSkIk9UWiz1jNxjz1EHatZGzHTHeLWL+Nx0e1HcMiB8BC/4bzfwu77PXBg4MRm6EAyDNRpfw dLeQnDEw==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rnKtR-000000074eD-1uQ4; Thu, 21 Mar 2024 16:03:49 +0000 Date: Thu, 21 Mar 2024 16:03:49 +0000 From: Matthew Wilcox To: Svetly Todorov Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, gregory.price@memverge.com, wangkefeng.wang@huawei.com, akpm@linux-foundation.org, david@redhat.com, vbabka@suse.cz, naoya.horiguchi@linux.dev Subject: Re: [PATCH v3] kpageflags: respect folio head-page flag placement Message-ID: References: <20240320-kpageflags-svetly-v3-1-b6725843bfa7@memverge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 6D6E2140046 X-Stat-Signature: a6zjuexz5hozw3qrm93w3sp99mrj1d4m X-Rspam-User: X-HE-Tag: 1711037036-610893 X-HE-Meta: U2FsdGVkX1+xB0YLw1zWieyVXhjloLglzb5WSUvxUbueqg3dkbX90AMATjfBOJZ0uHpM09VoAvJpMphnNMe7lgBlnQBVoSkFIW7t6BC/06PSj+VpNHZGFv2oed6TmZrTH658giSyW5iuYIV/iK8QhfFff2co+o6ikNlRPo6vLbhRGWqpz3BJr3EcxKHDTc+uYMsAkoEoEqTjw4D3oBqouszNjL0TG+5NxepNML+Ji0Z/9t+psI2H2MGNl/kqOZvjdE/bWAL8bP8ZV3IwtZu31SllWPiKr3Lj2C/0XizXTumK4zuDLev88JgqmqyTweEFrm20XIgYNmCaT+TuUJoND9oyJoTxKqx6+hrGr8DbeNL133U1L6tu1dx0sK+ODmb3nRlGNAc2G/D3Gw26+1c0ecuoxXjNUSag/5XesFBgEniprpFSb/z+1oTpfAwTMTulmkrY+2KWBhAg/UyhNkYLb9jlTiiRJCvrbYqqH8zkoPPfg3u2iCTs9Q5eSNmPr5198qW8wATYjZ/JzQmOwq22ndYMCJ5nWqN5RF1rVvd3D5Q4kc86ohAO7BRxc+P0DjlCR78mvZnXUI8RveuBc5qCh1jCLIiSkMcGp9B/PomcLqFAeYlJn9to+FhhC8o/QR7lFgBQ1KAKbENUUj+UgbpMZ52jtd9BODaLabZuQVHswloI9KSvTmrHiDwWYstmYB6t2Yw3L8fGgEOe9kcxoFvGjo7LT9lXDIqVco1xPItZZjOddY4y0ZS75/DTbiV5vzIEF/mY5k4LRonaTHXxqX5rQcMCfxjQltIhl7S1+jF2jlgYof9XX+HkKtT3Co+XabCyzzVmXzNyKLFkPFCY1hUyJQ== 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: List-Subscribe: List-Unsubscribe: On Wed, Mar 20, 2024 at 04:40:43PM -0700, Svetly Todorov wrote: > > Hi Matthew, > > > I have a somewhat different patch for this. Let me know what you think. > > It depends on a few other patches in my tree, so probably won't compile > > for you. > I don't have extensive experience with folios or anything but on the > whole it looks good to me. I like the use of `mapping` to dodge the > compound_head() checks. Beyond that, only a few things caught my eye. Thanks for your careful review. > > - if (PageKsm(page)) > > + if (mapping & PAGE_MAPPING_KSM) > > u |= 1 << KPF_KSM; > This might need an #ifdef? > Say mapping is movable and anon -- then (mapping & PAGE_MAPPING_KSM) is > true. Before, we called PageKsm, which falls through to a PG_ksm check. > If !CONFIG_KSM then that flag is always false. But now, we're liable to > report KPF_KSM even if !CONFIG_KSM. I'm not sure where you see a PG_ksm check: static __always_inline bool folio_test_ksm(const struct folio *folio) { return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) == PAGE_MAPPING_KSM; } static __always_inline bool PageKsm(const struct page *page) { return folio_test_ksm(page_folio(page)); } There's no such thing as a movable anon page -- the two bits in the bottom of the mapping pointer mean: 00 file (or NULL) 01 anon 10 movable 11 KSM Perhaps it might be clearer to say that anon pages are inherently movable; the movable type really means that the reset of the mapping pointer refers to a movable_operations instead of a mapping or anon_vma. > > /* > > * compound pages: export both head/tail info > > * they together define a compound page's start/end pos and order > > */ > > - if (PageHead(page)) > > - u |= 1 << KPF_COMPOUND_HEAD; > > - if (PageTail(page)) > > + if (page == &folio->page) > > + u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head); > > + else > > u |= 1 << KPF_COMPOUND_TAIL; > This makes sense but it'd require changes to the documentation. > I ran a python3 memhog to see if anonymous pages are currently reported > as COMPOUND_HEAD or COMPOUND_TAIL and it seems to be a no on both. > But with this, I think every pfn will have one of the two set. > Unless you can have a page outside of a folio -- not sure. I see your confusion. We have three cases; head, tail and neither (obviously a page is never both head & tail). If a page is neither, it's order-0 and it is the only page in the folio. So we handle head or neither in the first leg of the 'if' where we set KPF_COMPOUND_HEAD if PG_head is set, and tail in the 'else' leg. > Also, in > > - if (page_is_idle(page)) > > +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT) > > + u |= kpf_copy_bit(k, KPF_IDLE, PG_idle); > > +#else > > + if (folio_test_idle(folio)) > > u |= 1 << KPF_IDLE; > > +#endif > > > and > > - if (PageSwapCache(page)) > > +#define SWAPCACHE ((1 << PG_swapbacked) | (1 << PG_swapcache)) > > + if ((k & SWAPCACHE) == SWAPCACHE) > > u |= 1 << KPF_SWAPCACHE; > > u |= kpf_copy_bit(k, KPF_SWAPBACKED, PG_swapbacked); > it seems to me like the #ifdef/#define could be supplanted by > folio_test_idle and folio_test_swapcache. But I guess those would > require extra folio_flags queries and an #include . > So if this is more performant, I can understand the design. It's not so much the performance as it is the atomicity. I'm doing my best to get an atomic snapshot of the flags and report a consistent state, even if it might be stale by the time the user sees it.