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 B7B2CC61DA4 for ; Thu, 9 Mar 2023 04:59:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C0EF96B0071; Wed, 8 Mar 2023 23:59:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BBEDF6B0072; Wed, 8 Mar 2023 23:59:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A865B280001; Wed, 8 Mar 2023 23:59:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 966996B0071 for ; Wed, 8 Mar 2023 23:59:15 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 71D1D404A4 for ; Thu, 9 Mar 2023 04:59:15 +0000 (UTC) X-FDA: 80548155870.04.1A1D29E Received: from mail-qt1-f181.google.com (mail-qt1-f181.google.com [209.85.160.181]) by imf03.hostedemail.com (Postfix) with ESMTP id 793D920004 for ; Thu, 9 Mar 2023 04:59:12 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=3ync1xHx; spf=pass (imf03.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.181 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678337952; 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=u7Q758EVgylTs75aXHGbh2DNnGqg/9Af+1g3WINt/FU=; b=iGqAiEPQWbmx3vUu1MUQWOVpzYxhIAGILounfIcQAXPmMELo7st0KBPDmj42WBjvLUjkkh 3baBqt9NizmddZg42vihVJGI7RuIc9ymBl2RHXqiEnpDZBDKP6lovMPd4wC6HtRMps98l1 xE8kVoRC3nxWj6fOLBwBmXf8mLHlBR0= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=3ync1xHx; spf=pass (imf03.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.181 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678337952; a=rsa-sha256; cv=none; b=7T2zXiABKmTjX5F4K9OHW6/EwrXL1q3LopJgjfXG2APS/6O8P8xK34cN+Ifhx73Ffod1vs mrESaOCxJ0BBvUjlNbUxiaEYwrvWIjOvDff9O7AGN1WV4s6kimO9kBuxYAn9fzmM3ySnxY eyxhhxDlMUjRKe8V+BGQAgdpYPen7o4= Received: by mail-qt1-f181.google.com with SMTP id y10so887378qtj.2 for ; Wed, 08 Mar 2023 20:59:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; t=1678337951; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=u7Q758EVgylTs75aXHGbh2DNnGqg/9Af+1g3WINt/FU=; b=3ync1xHxY/YBYVd3IxyroCZqUb/GWTTYzZ8PELadIq/fDhfg2coxb7kzHgPX5H2uPP 97pkKNtMLAIOTy7KS4xl8BRtfUTfXZGTeXaz1eBkHfQ3hxiYVeZapniGPMiCkLtQU+UI 4tnBNI9JVwxGld8T5h4FbOXrAb6MEZNKErtmlCLZ8E70PlymYUVVc0NsaHhvuScDNh1v Vrh+7q08WKE5N2tu3HCooIUlv4S+HbzlXxPH9sgA/QXQqTGHU3pP30GAG8Zj9Un8j7Ok IFsGCaZxpEkqJyH3FoaLxlQuiLQCVIevF1e8yH5CK5GwI+Jdv0siKEat2sGj4lVTA1kd uetA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678337951; 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=u7Q758EVgylTs75aXHGbh2DNnGqg/9Af+1g3WINt/FU=; b=6Q7bkhCQpDCVLfUa1c7gzFYUrXvjSkXMEvmS44mwZ40yqEMfuYoJ4Oclk62sHwWZKx b3PEOWnVk31lFTESrNTocuhEO0CfCNqcQBBpOfB86UPeqc7sY5BwhrMXHUnZbkzb/yhm Ocbngsz1+demGrfCdfGUsu9yFkv48sZeWsRWcCDGx8e/3hug/yrUMdzNYlQ1rn3eBc73 XG3bB+8h24G9X3n2Cox07LfIOc3zq+znhwV05oSQXTaD0jX2Fi+Mhk4371msFKvQtFU6 THSsDG/nBwaIb6IpLQhgxsPS5JvUoHoZwqYTuPK4GW6+nMGVytZ19ZcEvGjUq8vdHAn+ wpzQ== X-Gm-Message-State: AO0yUKX7VetWzcs2cxw55D9uxbmJDK/IhAr9mzURxZHP/ri7wdQymoxm y3W14BUtwDtYwYo9V8AraJX6cw== X-Google-Smtp-Source: AK7set8HRIUSrbF4IGC24O6vqnaFTBCTpfvzE95pYf2T5AMVM0aMH3GWnuSEGSNHcoiK/ACjtMjm7w== X-Received: by 2002:a05:622a:170b:b0:3b8:58d0:b4e4 with SMTP id h11-20020a05622a170b00b003b858d0b4e4mr2431886qtk.33.1678337951507; Wed, 08 Mar 2023 20:59:11 -0800 (PST) Received: from localhost ([2620:10d:c091:400::5:d32c]) by smtp.gmail.com with ESMTPSA id e8-20020ac80108000000b003b9a6d54b6csm12913588qtg.59.2023.03.08.20.59.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Mar 2023 20:59:11 -0800 (PST) Date: Wed, 8 Mar 2023 23:59:10 -0500 From: Johannes Weiner To: Stefan Roesch 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 Message-ID: <20230309045910.GD476158@cmpxchg.org> References: <20230224044000.3084046-1-shr@devkernel.io> <20230224044000.3084046-2-shr@devkernel.io> <20230308164746.GA473363@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: dzobmb5trb9ebqwyctdabj7u9x6t9m5y X-Rspamd-Queue-Id: 793D920004 X-HE-Tag: 1678337952-945925 X-HE-Meta: U2FsdGVkX183J0x9XT/q/M3Y5dxG5ZcshsdRc+/2tzQPi4M5yy0QcFk1yhxKQAh8W/qWeAGOka+2G6XViJxcq2LU4pYtWbChfR1txsFbmI42Xkxy4T8xqpJWqKW7Hkd+ehY1yf6kKvQmMlIJmz+9n5hXRCr2RWSPoZGSce/Vxrhq26sQ1GDs5FcdGfjbXEknaMgGAjTaRuLJo4tyCLOmN4SPdafNPXtscoR4wJsov2k8uocdnu53Kzg3COkI+ILA3UIi+LyffWEAowMsO9aobBl0rXQ5fq68lHI/N5ryqaAJT+SH54hNPQweuyJHPEem3bhXy7j/reU6Wg6PMwfHbhYItJ0DSDbFDWK729hbGOGjd6mfTbO2RPQADbXUDkOVFdezJcPrmX8hOotXuAsKGMdpFK9gWh7d6MV13Dco3CFsPAbtOjzAqklwfak2Y1Fsrv20e3e0dyK8QdkM1xJ3bqR40t5sgAcowW/YXjy5CmzLpNl+fhPk4m5dFcUP3poXaDJMoOI0Phw7yN9CYIP7UQr1xx+N+KIN49YOknCafsR8C0mr+houmVGLq7fcQ60xHvOOw1o3wf3+tDndtI7OGBGUHkVIYGIeHph37W5QQPZ/gdD+OHcPgFW3/61CLWgtEMeOjQRXQ/WT7/JKdJHAoEvUXym+38AZkM1TssCUc99f+GPjTVfOTXtWCeELtbzAXwYn6SgY7YqD7s2uWsddoPU56xpBPaNJMZRMVWK4KMW82FOh83qQQA/DpoACRlTNij25PJRLfvMOqq9ySZr4WEqL+ffvlnr9KMgrr2Cb1lCt/qZ0AB8/xKdp5iljWJlVi31RoIK376OZpJxdHQJimIXvjfsfvdIR3GCBDlKadS6MR4I5fmbVuuIpeo0CEcaNwzq/IJIBpTKmy3oaney7QMNluSyCguunu9sgqLS1Zg60TespEExIYh55g+ZMGq8kyIpJQgFTKDhZtE+gwD9 TbdPp35M JEsLKpcQkrSgJRhShP+ieo6yhD+aTasaW/a2WnTA2uv24TVyLS78wGs53Kp8miW4b8Y3Jca92i1urzW34mKKUL/Uoti+rHYdDsxFaFY09oPr9PTaNXvQmYIcXGkqU/4ICREO14Wc6Z67rdpH/RnSNS2NjlIcQZyPPJAdj0y/SwWPQWkV/viQfm0G7CGzajH6q+ljyU2O530ZaiJyWSOVYDcgEznPX6yk4//rG5vNkn4E0eRQY9J4BV4IK+bYh1R8NaHW+qG2QLizTbxVy5CHc+bjPlwGwUSOJ1KsgNmw3QKmWK0FVjCzbELU3r9miBmqbSSpNPktRcUdHxw2HujtbFKht5FRhxS/tqZeU/ufVek4hJtawMfFVSfYdTDyoaksEBYO+pO8D7OQJWMzcLX7tUMjHelj0BA58bpRhVWWbXyohh4qinEcI4ZxEPQ9O0q5T6ECzK/FbvurymJzii/3FM8rce1LuEOKIsiir4EiLYj9kNx1CtgnNpVDP5yTEZo3CWV/2QbCq/Qu1r0A= 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 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. :)