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 856CDC05027 for ; Tue, 14 Feb 2023 16:58:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1EDAF6B007B; Tue, 14 Feb 2023 11:58:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 176DA6B007D; Tue, 14 Feb 2023 11:58:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EE5236B007E; Tue, 14 Feb 2023 11:58:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D82C56B007B for ; Tue, 14 Feb 2023 11:58:57 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 780E51C67F5 for ; Tue, 14 Feb 2023 16:58:57 +0000 (UTC) X-FDA: 80466507114.29.82D4BFD Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf24.hostedemail.com (Postfix) with ESMTP id A4686180011 for ; Tue, 14 Feb 2023 16:58:54 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=EyYLerSo; spf=pass (imf24.hostedemail.com: domain of david@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=david@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=1676393935; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=kuHwzZTrvbVLrZIEtXxaNZLPc/tFsdLuSJJ5efA8PZ0=; b=NeIRPI3NhnqvkS7ism/jWCsDpeJlb7kczfd1xVYDjecjGfAKqpMVdvSgSg7iLy+y4wJRbb ALjXswwItytuHNrSVN89/lGvSWY6QAwR8PVlBKT9T2T7SfjyocKQtRr17WZMrW8KTGxpPH wN8LmqVXiW71ao2anbJINusy3zHm1XY= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=EyYLerSo; spf=pass (imf24.hostedemail.com: domain of david@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1676393935; a=rsa-sha256; cv=none; b=6pt2CLyeFJf3qaqdm1ibWXpFgqORtESfRFa9NuGTWOigyJAtAZ9Vuls09cdHosXDVQwhBo sxonuOoXbw4Ab5e3Qtp2LcZ6V5K132ZLYE1BOoiswkl5DSvgGRGtbDouk00nXOrnxQhWVP dbZrEPGRQs8ianslRUiHG8xpBEs5h6A= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676393933; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kuHwzZTrvbVLrZIEtXxaNZLPc/tFsdLuSJJ5efA8PZ0=; b=EyYLerSocADZG7n3Ny+8a5mpmKiYkmqOv4XDNcz8tMsub+1GPmrqNnJ3Z9eLBJmHgz2bR2 /kA0wpQgh5U4u7N0iURmdsrj3NkxjuXIt0B3wk+54pb7qez41ANjEG9r43SaEGgcjFYIH4 U2rG2iV5oyOxE7abW3KDZM6Eh/pHSbQ= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-528-xp6L3M8AP62-FIv2qTTNJw-1; Tue, 14 Feb 2023 11:58:50 -0500 X-MC-Unique: xp6L3M8AP62-FIv2qTTNJw-1 Received: by mail-wr1-f71.google.com with SMTP id n18-20020adfc612000000b002c3d80ab568so3197633wrg.18 for ; Tue, 14 Feb 2023 08:58:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:organization:from :references:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=kuHwzZTrvbVLrZIEtXxaNZLPc/tFsdLuSJJ5efA8PZ0=; b=4c+to8SVkKmG+jjHO/6YaWB7ixRSKK99/m56X/FWvA6HRkrJi+NwNYb/ToVwYi2WS9 /dn9EdfuckwXZWLV87qnXXNfRxMYBXyeXy5KZLFBJQw11TBTi++aa9C+T4ImjvUyQkCH 5+qM6IrGvJEsV6ftLLWeosCx/U9GhLAQQwAkwlnolnlp6JwRrnPwnNTHowOYBUkdg7IS sEFFxi+9WZUhrecWj20At/zbafZHdsq1RxgIfP5xIGAIwN7EQyM9WDiOgSwmkpNhiTWe bGhmj/v8g4qL3OjHNW/F+XSzbTeg8S+06GP8OVLXaKsGx5vainsrCR8ITyRdrEuWM9BX STIQ== X-Gm-Message-State: AO0yUKXOotJ7fYKIipkB30CUlI+Schwfah7Et3Wnnopj6eMd+g8bi/EY vVsS9PRB1GEeVuyz9F7hmIwX/++VdZWVvsgq/SnbdXXKcIrERLD3+Tr/Ptctker5aCbca4HnC4S QQYztFk/rxRg= X-Received: by 2002:adf:fd11:0:b0:2c5:60e2:ed6b with SMTP id e17-20020adffd11000000b002c560e2ed6bmr2472197wrr.2.1676393929306; Tue, 14 Feb 2023 08:58:49 -0800 (PST) X-Google-Smtp-Source: AK7set+c46SGb0cyt7nBCeLUBmB3MfNMmWwOtQ0u0jB0emQwNtfrzpIlWNiHXf7awUpb3XlyQknNdA== X-Received: by 2002:adf:fd11:0:b0:2c5:60e2:ed6b with SMTP id e17-20020adffd11000000b002c560e2ed6bmr2472156wrr.2.1676393928992; Tue, 14 Feb 2023 08:58:48 -0800 (PST) Received: from ?IPV6:2003:cb:c709:1700:969:8e2b:e8bb:46be? (p200300cbc709170009698e2be8bb46be.dip0.t-ipconnect.de. [2003:cb:c709:1700:969:8e2b:e8bb:46be]) by smtp.gmail.com with ESMTPSA id l18-20020a5d6752000000b002c5503a8d21sm8549944wrw.70.2023.02.14.08.58.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 14 Feb 2023 08:58:48 -0800 (PST) Message-ID: Date: Tue, 14 Feb 2023 17:58:45 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 To: Chih-En Lin Cc: Pasha Tatashin , Andrew Morton , Qi Zheng , "Matthew Wilcox (Oracle)" , Christophe Leroy , John Hubbard , Nadav Amit , Barry Song , Steven Rostedt , Masami Hiramatsu , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Yang Shi , Peter Xu , Vlastimil Babka , Zach O'Keefe , Yun Zhou , Hugh Dickins , Suren Baghdasaryan , Yu Zhao , Juergen Gross , Tong Tiangen , Liu Shixin , Anshuman Khandual , Li kunyu , Minchan Kim , Miaohe Lin , Gautam Menghani , Catalin Marinas , Mark Brown , Will Deacon , Vincenzo Frascino , Thomas Gleixner , "Eric W. Biederman" , Andy Lutomirski , Sebastian Andrzej Siewior , "Liam R. Howlett" , Fenghua Yu , Andrei Vagin , Barret Rhoden , Michal Hocko , "Jason A. Donenfeld" , Alexey Gladkov , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Dinglan Peng , Pedro Fonseca , Jim Huang , Huichun Feng References: <20230207035139.272707-1-shiyn.lin@gmail.com> <62c44d12-933d-ee66-ef50-467cd8d30a58@redhat.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v4 00/14] Introduce Copy-On-Write to Page Table In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: A4686180011 X-Stat-Signature: h116brzqof4ytuyszt43kaftqkxaj957 X-HE-Tag: 1676393934-732851 X-HE-Meta: U2FsdGVkX1/NrFELg0ZkULpFIu9hwCnnETrP+0e0ad+U+H8eIULM6jKA210dgEu3mWvldgPpgtuDiLCG2oQcOmagk1mc+GMFFtiFSJeM6sdvjM/ZDNRpemEEHyyICBg8makacIgu9b3sd6zurWppbJQOgmcE78tfsLa0BHUrQ9XIZNEMozySfWTyd0qBEm1fnQjKIf2R6AdSUbtvt+q1C4rBmhNCd5C8uJdPbX5wVK3srADf8jI0p6DKlauhEx2sBximjKPBRJpMoc4gEKOC2J0ylz5Nrcv3FwIRND4T3iPxkcLwvXQ/Ctk4ppgVb4HXr/ywV/V2LXamIREo2aBSu63VNFc2LSIGjhuhZIAIkwl24Y4WT6FHqMTWAK4u/CsdMq3UbCZtaRwstSlbDBn8MoLqt83IU5wiPF4I4djqzyVWt3Z6hs9Kx6+e0d/z9wU9NRieaHf6Z4INIYRskwioQ0+OSPGq1kXxa0EtnDGH8louNlB6xKHAQeK6PmIq6/A0pH0y3mRFdaTbBfK5NElFePWZOOdfQHqCvITllWXtqw2Swm2Si8ylbTXAF8oekFwCP4Ypl3zHfGD/ZEPK/JK5WQZrjekyzii1c89BLZuVZWRY1d27YFDS8SUpx6eJGFy4ZXIVH+1EtcmJ66AvA4MCyQznNbN9OJknale0R3qcx30iM7wr02+uNnzf1kR0CP4l7Y76zCw8UqkeqUkiAil4isCpYzV7VLxP0BXAQIyqu4HcNuf9zy0jkvWrnKUvt2gnzgTT+oCAPk+9y9xBCfZ9cxM6/Bho1z2l0XAd0qgVSzAejUC3PBZN+5M23OxNVBo4hzmSu9AXYyXVLrSAkDQ9qWIEvqGEs7M0XP0GSIc9mHUhmmlrSLjzmwY64HNgWCvrcg+hl6dH6Cmabnxam8e83ntOgolr9ow1195nh0/flmkl8SZ6xOuXyPTHDU4E9ADpuqhgb4jvafBddSb2Mr4 Y11W5y3m S4lWu7DRLiJyjZJOtDR7X/G1KYWP/az8roacLo4S86coJLk4AOiEkSrrLl+9f4dQJ4r1DRrsuHSw19CpKtOaWjrE0IKQtuOZLBor49biqWksHacl8wSVK5JEEjLy4569rQ2qd7zPgtH+hxp/UXVYsauMrBrm/3FypM56Xp+zi5A64F6vocgijiz6R4p/GadwG7yhEOBI53eXezTkIavoamayxtEEMxb9ahudNtRoUf2YwphoububQDIr0aWT4Tflela2jTNuHcp4c4Ez+kLmX4xJ0vHqIUVrPDgVVO+i7QUi2FueFYIxkoRAo0ObCgBpQS6bsLatgUSbSiyVi4AKmdjCEgysKYcr0e7LcrkNy6GuXLPc5e64XpqvgBnjoBskIVSyCCqSgunnBtih5/lGTEbC+h5iceVbUDZ3q9RJq06Vm1eUGiBjEwnkzTQrUS5Mh5fZrLOYoIJxlH46jh7tfStGy8hlaaY80fF2UAZYiHt0Y9xqN8upVIYX3hnuIBeiGwcSgJz/AQSCoqNV5tU4YsoVBg+M5MezQsydT3YNgnQ7Hg2rNsM8RtBYD55q+qDdDKiY9oW6zqyS/7OQ= 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: >>> >>> Honestly, for improving the fork(), I have an idea to skip the per-page >>> operation without breaking the logic. However, this will introduce the >>> complicated mechanism and may has the overhead for other features. It >>> might not be worth it. It's hard to strike a balance between the >>> over-complicated mechanism with (probably) better performance and data >>> consistency with the page status. So, I would focus on the safety and >>> stable approach at first. >> >> Yes, it is most probably possible, but complexity, robustness and >> maintainability have to be considered as well. >> >> Thanks for implementing this approach (only deduplication without other >> optimizations) and evaluating it accordingly. It's certainly "cleaner", such >> that we only have to mess with unsharing and not with other >> accounting/pinning/mapcount thingies. But it also highlights how intrusive >> even this basic deduplication approach already is -- and that most benefits >> of the original approach requires even more complexity on top. >> >> I am not quite sure if the benefit is worth the price (I am not to decide >> and I would like to hear other options). > > I'm looking at the discussion of page table sharing in 2002 [1]. > It looks like in 2002 ~ 2006, there also have some patches try to > improve fork(). > > After that, I also saw one thread which is about another shared page > table patch's benchmark. I can't find the original patch though [2]. > But, I found the probably same patch in 2005 [3], it also mentioned > the previous benchmark discussion: > > " > For those familiar with the shared page table patch I did a couple of years > ago, this patch does not implement copy-on-write page tables for private > mappings. Analysis showed the cost and complexity far outweighed any > potential benefit. > " Thanks for the pointer, interesting read. And my personal opinion is that part of that statement still hold true :) > > However, it might be different right now. For example, the implemetation > . We have split page table lock now, so we don't have to consider the > page_table_share_lock thing. Also, presently, we have different use > cases (shells [2] v.s. VM cloning and fuzzing) to consider. > > Nonetheless, I still think the discussion can provide some of the mind > to us. > > BTW, It seems like the 2002 patch [1] is different from the 2002 [2] > and 2005 [3]. > > [1] https://lkml.iu.edu/hypermail/linux/kernel/0202.2/0102.html > [2] https://lore.kernel.org/linux-mm/3E02FACD.5B300794@digeo.com/ > [3] https://lore.kernel.org/linux-mm/7C49DFF721CB4E671DB260F9@%5B10.1.1.4%5D/T/#u > >> My quick thoughts after skimming over the core parts of this series >> >> (1) forgetting to break COW on a PTE in some pgtable walker feels quite >> likely (meaning that it might be fairly error-prone) and forgetting >> to break COW on a PTE table, accidentally modifying the shared >> table. > > Maybe I should also handle arch/ and others parts. > I will keep looking at where I missed. One could add sanity checks when modifying a PTE while the PTE table is still marked shared ... but I guess there are some valid reasons where we might want to modify shared PTE tables (rmap). > >> (2) break_cow_pte() can fail, which means that we can fail some >> operations (possibly silently halfway through) now. For example, >> looking at your change_pte_range() change, I suspect it's wrong. > > Maybe I should add WARN_ON() and skip the failed COW PTE. One way or the other we'll have to handle it. WARN_ON() sounds wrong for handling OOM situations (e.g., if only that cgroup is OOM). > >> (3) handle_cow_pte_fault() looks quite complicated and needs quite some >> double-checking: we temporarily clear the PMD, to reset it >> afterwards. I am not sure if that is correct. For example, what >> stops another page fault stumbling over that pmd_none() and >> allocating an empty page table? Maybe there are some locking details >> missing or they are very subtle such that we better document them. I >> recall that THP played quite some tricks to make such cases work ... > > I think that holding mmap_write_lock may be enough (I added > mmap_assert_write_locked() in the fault function btw). But, I might > be wrong. I will look at the THP stuff to see how they work. Thanks. > Ehm, but page faults don't hold the mmap lock writable? And so are other callers, like MADV_DONTNEED or MADV_FREE. handle_pte_fault()->handle_pte_fault()->mmap_assert_write_locked() should bail out. Either I am missing something or you didn't test with lockdep enabled :) Note that there are upstream efforts to use only a VMA lock (and some people even want to perform some page faults only protected by RCU). -- Thanks, David / dhildenb