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 6888CC74A4B for ; Thu, 9 Mar 2023 22:38:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0B9286B0078; Thu, 9 Mar 2023 17:38:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 06A79280003; Thu, 9 Mar 2023 17:38:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E7393280002; Thu, 9 Mar 2023 17:38:09 -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 D73166B0078 for ; Thu, 9 Mar 2023 17:38:09 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 9D303120C7D for ; Thu, 9 Mar 2023 22:38:09 +0000 (UTC) X-FDA: 80550824298.12.8D84340 Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by imf27.hostedemail.com (Postfix) with ESMTP id 92DB940014 for ; Thu, 9 Mar 2023 22:38:06 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=devkernel.io header.s=fm2 header.b=TapyiyUj; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=VAbCLuqQ; spf=pass (imf27.hostedemail.com: domain of shr@devkernel.io designates 66.111.4.25 as permitted sender) smtp.mailfrom=shr@devkernel.io; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678401486; 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=Ze3+pxMevBT00ZvMFDroeTtBvnMDSJcEIMQQMq34NOo=; b=0EWsxkdGfv/kQX64JI/COVetBjVKdGfqWiwkD8xZJ6Rgnj/BAE16UVjciezy3r8Hz8IIYR oUPuO08WcHh459WpAh1dPSFSXw1jfWWzjVG/DG2lw3QudiFNMvWWhWRr3F+7Fk5y7wMusZ zsg7nk/aYGi3tD1t0T0DCU3jNHTyHIA= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=devkernel.io header.s=fm2 header.b=TapyiyUj; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=VAbCLuqQ; spf=pass (imf27.hostedemail.com: domain of shr@devkernel.io designates 66.111.4.25 as permitted sender) smtp.mailfrom=shr@devkernel.io; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678401486; a=rsa-sha256; cv=none; b=69E+i8Vw3IPvD8yFFEO6Hm77Eqz4/TdthTJgRn+Oyfc5RP6he/xM4ZOeSYifuF0c1+0ZU1 si/2CccX1YOzXx7NuLlCJbYNH0gNvaq8I7D5yy04NBcEewyA2aR/kRyt+O//ZIjRXJ0LB+ tqL8s+uBl82rd+QHc9rHVuvxD6drCL8= Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 166145C0166; Thu, 9 Mar 2023 17:38:06 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Thu, 09 Mar 2023 17:38:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=devkernel.io; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm2; t=1678401486; x=1678487886; bh=Ze 3+pxMevBT00ZvMFDroeTtBvnMDSJcEIMQQMq34NOo=; b=TapyiyUjWS8nJHLJYu K4MTpsfTjdpBVaVfg5m8PuG+bJIKFuCHys4P79nNp7gXznmRyzk9AiTLkqzdscPA fxF230oU/MLcftTC0gddvvDRIJmuHWo5inXinR9Z1NZ0NbEcJf5OZOG9dcqbWXT3 up4CvBTawMj4f5XGY/6z3y/9GaOYH++zCNfRQXTCHp/2ImTqxuANBTsV0UJ4xc4z vxwjiAILFey1ZBZ5J/02MfJ25MSwNvDaKBzc2BRxMdHNkAx+X+U8PmIj7sLh1gzN WXkAWcEa3HB9dH+MPBFh8N02TaKWjZipOhwf2v6owgRyDeCFj0mIi+Jx75D+f+oi kliA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1678401486; x=1678487886; bh=Ze3+pxMevBT00 ZvMFDroeTtBvnMDSJcEIMQQMq34NOo=; b=VAbCLuqQDWyXb8usdEton5pImuh9i p66rVmg5ehzX8siKz7T3v41LDXIEOqvIfhJueXXpPa2KX+T6dpHGgNK8n8zbcU87 NoxYnTzqdiOPaEv2X/YiKJgnTJFS3v+GkhxYQp1RlvqoXzavsjvlKMnRVL96kw2M MEl1Iz51kvYyK2VTtMG0iMtQ7LaeFf+2lVlj1xOVlmjMq+QytPCZeRSniktwBcYK UicLAutRmf/7nyvdXb+I60L3fpQ8RK88gAd/R8YNsyCkMeAOmHfhWP3zDQimTd/3 1fNT/AzBiXRZjqBQ+Dn0NfUF+EHKZsdVmfJ6jpOrCzGkP/OeFJl0M/A+w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdduiedgudeitdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfhgfhffvvefuffgjkfggtgesth dtredttdertdenucfhrhhomhepufhtvghfrghnucftohgvshgthhcuoehshhhrseguvghv khgvrhhnvghlrdhioheqnecuggftrfgrthhtvghrnhepveelgffghfehudeitdehjeevhe dthfetvdfhledutedvgeeikeeggefgudeguedtnecuvehluhhsthgvrhfuihiivgeptden ucfrrghrrghmpehmrghilhhfrhhomhepshhhrhesuggvvhhkvghrnhgvlhdrihho X-ME-Proxy: Feedback-ID: i84614614:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Mar 2023 17:38:04 -0500 (EST) References: <20230224044000.3084046-1-shr@devkernel.io> <20230224044000.3084046-2-shr@devkernel.io> <20230308164746.GA473363@cmpxchg.org> <20230309045910.GD476158@cmpxchg.org> User-agent: mu4e 1.6.11; emacs 28.2.50 From: Stefan Roesch To: Johannes Weiner Cc: kernel-team@fb.com, linux-mm@kvack.org, riel@surriel.com, mhocko@suse.com, david@redhat.com, linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org, akpm@linux-foundation.org Subject: Re: [PATCH v3 1/3] mm: add new api to enable ksm per process Date: Thu, 09 Mar 2023 14:33:10 -0800 In-reply-to: <20230309045910.GD476158@cmpxchg.org> Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Stat-Signature: d59nrpggtb1iuyugewb6amdnu11rhsfu X-Rspam-User: X-Rspamd-Queue-Id: 92DB940014 X-Rspamd-Server: rspam06 X-HE-Tag: 1678401486-295583 X-HE-Meta: U2FsdGVkX1+kvC3IrS4rbZCGJdMMRRN9/a3xEo0XkXimRjLU+37qpH4uUErXj1SWrGSMxXM/tSgYehs7XLbSsLImmuy0S5N8hNbh9BGtlWh5nA78UYnipu9b4fVazQZEITaYcZG5AkcSZmGM4OK4g5Vg4ajpCIHw5h4/TrQZYMcnHfWKDndp01A4UKFBiSidSH82U2TUL4K7xkTHVAJrSNQEupFiocULZYHL2RcbAuMHUApioooaxqr4MQ8esTPM9NbokLyjU7ecv+yPczvWw7xO0KwrAOsD4tMVyfYsaMC3DLzz9sUIZLqyFKtJJVZrIO4MKsBun8TBA/gpvAzQWIsTNGKp6YdX068tR+4LjrwmAWx9AwvdZ7l2WNsi38/dyUpqafwxDPPcq9smSC4h500m2msGdXdIEXm+mJiCJbnEHwyZDAEj8P2bFe7X/9WxI4JF72Vze+zy2nkO3dR5rBDt7LzJNwI1YCr88Sw3fFD8BxmkpSSyBmryL24cGHye6DCmyz8ltgetgiZzg7yLRaO+TnuHOiDvvIWBbSv1feSjgtpsKqr/tFwtNLzE9cQUgC8po3pL4K7lr7vCpzOgMrJI5VjB0kBPxN66jhWepls91UoEevZIKR8HGzP2HNYpc79Mp19X7E0alQsc7Zz7Srkw5NJzj96E067qYYKsX6fwAWaB+fh6ODzEixrssF/1mTB4ENRuiFGq9Ywu3yIGFAJsuoDb2QRYwLWWyKlxmMKQw0az/y/Z+8cNTiVWNb39pS+lbE4OWSxQxzJ7JBQvlsTwdsxPfzM0sGgQNk729qUW4eETcm5cavFmt065xXdhkJE4xul8cP3LYQJEzhwKjZtJC+iKYQMrkA85dxzb4Wj94Ro+iKN51jyKx+9dA/qwGb0TAOC4Ts5YVLk2LhmBFB9CdVSTxOFKN8QmWL6y499zBUFE9gZuRMNl4ayAaq4b3TJmZ6vFVvn27YH5f2r WKofLHaZ S//Pia1T4/3YK7+GXcQzLpC9TNbgwZmAOdh6WEMUXV8lBowxyNJsN8fNCD5GeQKZE/JM1bTwkLNewWxp8iNDOUpGiuP8dKRW7odI2L0AueSpSaDtZhN01aVnRTrCdlIWaeXaoI7YxO2SkdS6PNTUBEGLbChyrfHR1jsUkSauoMl/7g5EEusywRqvsFuyc/aS/9XL0Ycsolaihqmb2ZZFuundUg2GzK9ohHO051ThJlj8+gl8= 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: Johannes Weiner writes: > On Wed, Mar 08, 2023 at 02:16:36PM -0800, Stefan Roesch wrote: >> Johannes Weiner writes: >> > On Thu, Feb 23, 2023 at 08:39:58PM -0800, Stefan Roesch wrote: >> >> @@ -2405,8 +2417,20 @@ static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page) >> >> goto no_vmas; >> >> >> >> for_each_vma(vmi, vma) { >> >> - if (!(vma->vm_flags & VM_MERGEABLE)) >> >> + if (!vma_ksm_mergeable(vma)) >> >> continue; >> >> + if (!(vma->vm_flags & VM_MERGEABLE)) { >> > >> > IMO, the helper obscures the interaction between the vma flag and the >> > per-process flag here. How about: >> > >> > if (!(vma->vm_flags & VM_MERGEABLE)) { >> > if (!test_bit(MMF_VM_MERGE_ANY, &vma->vm_mm->flags)) >> > continue; >> > >> > /* >> > * With per-process merging enabled, have the MM scan >> > * enroll any existing and new VMAs on the fly. >> > * >> > ksm_madvise(); >> > } >> > >> >> + unsigned long flags = vma->vm_flags; >> >> + >> >> + /* madvise failed, use next vma */ >> >> + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, MADV_MERGEABLE, &flags)) >> >> + continue; >> >> + /* vma, not supported as being mergeable */ >> >> + if (!(flags & VM_MERGEABLE)) >> >> + continue; >> >> + >> >> + vm_flags_set(vma, VM_MERGEABLE); >> > >> > I don't understand the local flags. Can't it pass &vma->vm_flags to >> > ksm_madvise()? It'll set VM_MERGEABLE on success. And you know it >> > wasn't set before because the whole thing is inside the !set >> > branch. The return value doesn't seem super useful, it's only the flag >> > setting that matters: >> > >> > ksm_madvise(vma, vma->vm_start, vma->vm_end, MADV_MERGEABLE, &vma->vm_flags); >> > /* madvise can fail, and will skip special vmas (pfnmaps and such) */ >> > if (!(vma->vm_flags & VM_MERGEABLE)) >> > continue; >> > >> >> vm_flags is defined as const. I cannot pass it directly inside the >> function, this is the reason, I'm using a local variable for it. > > Oops, good catch. > > However, while looking at the flag helpers, I'm also realizing that > modifications requires the mmap_sem in write mode, which this code > doesn't. This function might potentially scan the entire process > address space, so you can't just change the lock mode, either. > > Staring more at this, do you actually need to set VM_MERGEABLE on the > individual vmas? There are only a few places that check VM_MERGEABLE, > and AFAICS they can all just check for MMF_VM_MERGE_ANY also. > > You'd need to factor out the vma compatibility checks from > ksm_madvise(), and skip over special vmas during the mm scan. But > those tests are all stable under the read lock, so that's fine. > > The other thing ksm_madvise() does is ksm_enter() - but that's > obviously not needed from inside the loop over ksm_enter'd mms. :) The check alone for MMF_VM_MERGE_ANY is not sufficient. We also need to check if the respective VMA is mergeable. I'll split off the checks in ksm_madvise to its own function, so it can be called from where VM_MERGEABLE is currently checked. With the above change, the function unmerge_vmas is no longer needed.