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 BDDB2C6FA86 for ; Tue, 27 Sep 2022 19:00:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE0018E00F9; Tue, 27 Sep 2022 15:00:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E8F278E00C1; Tue, 27 Sep 2022 15:00:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CE1D38E00F9; Tue, 27 Sep 2022 15:00:23 -0400 (EDT) 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 BB7278E00C1 for ; Tue, 27 Sep 2022 15:00:23 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7D7D9C08FA for ; Tue, 27 Sep 2022 19:00:23 +0000 (UTC) X-FDA: 79958781126.19.F2225C1 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) by imf04.hostedemail.com (Postfix) with ESMTP id 7C59A40007 for ; Tue, 27 Sep 2022 19:00:22 +0000 (UTC) Received: by mail-pj1-f48.google.com with SMTP id i15-20020a17090a4b8f00b0020073b4ac27so10886793pjh.3 for ; Tue, 27 Sep 2022 12:00:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=VPxbWHE5oCoBASI+IJV2mNevW1WyTdk4e8pg764ECb0=; b=h6b35SB/HPmt+thde5SoMzdt5qQVqdbZHxdMkZDfsjj/Cc/ORwXB6FnxKTfoQ1XhbO Ru/Dsk8AGYm4E8NAUEb8ZxZeuTncC6ODN+LY4hKHjnw3MYFlKN5To820ND0rPHgaElmN JnCpx2XB49Ehy4tQcc5lAwcvtfWWsqurb9IjLYZGxfuxREgMzIYwT/VB0qH7yc7abKYo +yqvbadVJIVaBno4Qkoke53LEuDixrVhNljjqS711EqO+tlprIZCHsqJxJSfWoTPI1V5 77IiBsnnqhIesaYS+FSjCzgKwYeOdsZ9RO36+TD7dgf9m/A3rUCvInmdNInMfe3v+PHg VCOQ== 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; bh=VPxbWHE5oCoBASI+IJV2mNevW1WyTdk4e8pg764ECb0=; b=AK34RPv+xtEyh4LA5hQPhXS8ykBhtSUh3HebAWQGXsefySr1wQ2ropynrflqfNOKeH xuNH5H8w4YD39yakzZz2id58gARxS5pFjfCbC2qEPiMZ6Qt+YnrkTl+MnbQbAqomJa9h v6bk9IPfkxHL38ArcyxFof79MwoL4AE4SrX3rBS6Feq9u/dTgm6Hlk/kjJ73t5w5oJRh ReWDXwS6RkifWSZssbaQ9DOF0NVch+aiaCuLq6xSKUNlZGrlDsx7iieSlVklyntBqV9u /6T0yfV49Ak5X4W7KU8C2xDLBefc1Ljf2nbGoUp7UwGvhRsYST9nzIZYJPN0NVtDJjCv ZBFw== X-Gm-Message-State: ACrzQf3f7w256pcoo5FWz8jvEOND2T1ZP6NPnDUtQgRoKOhaoegHl6OX V14sMqj4MhFZVaN4GiuYfLI= X-Google-Smtp-Source: AMsMyM4y55/IcjlbxSTPZI7uXkcejWhcpYaJK2zT1ATKi287UNNNdHfidxyd7tsnLwmdv2vhikcdhA== X-Received: by 2002:a17:902:8a97:b0:178:1939:c721 with SMTP id p23-20020a1709028a9700b001781939c721mr28701536plo.108.1664305219879; Tue, 27 Sep 2022 12:00:19 -0700 (PDT) Received: from strix-laptop (2001-b011-20e0-1b32-d148-19d6-82fa-6094.dynamic-ip6.hinet.net. [2001:b011:20e0:1b32:d148:19d6:82fa:6094]) by smtp.gmail.com with ESMTPSA id j9-20020a170903024900b001708c4ebbaesm1856571plh.309.2022.09.27.12.00.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Sep 2022 12:00:19 -0700 (PDT) Date: Wed, 28 Sep 2022 03:00:12 +0800 From: Chih-En Lin To: Nadav Amit Cc: Andrew Morton , Qi Zheng , David Hildenbrand , Matthew Wilcox , Christophe Leroy , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , Luis Chamberlain , Kees Cook , Iurii Zaikin , Vlastimil Babka , William Kucharski , "Kirill A . Shutemov" , Peter Xu , Suren Baghdasaryan , Arnd Bergmann , Tong Tiangen , Pasha Tatashin , Li kunyu , Anshuman Khandual , Minchan Kim , Yang Shi , Song Liu , Miaohe Lin , Thomas Gleixner , Sebastian Andrzej Siewior , Andy Lutomirski , Fenghua Yu , Dinglan Peng , Pedro Fonseca , Jim Huang , Huichun Feng Subject: Re: [RFC PATCH v2 4/9] mm: Add COW PTE fallback functions Message-ID: References: <20220927162957.270460-1-shiyn.lin@gmail.com> <20220927162957.270460-5-shiyn.lin@gmail.com> <8041503C-D087-434D-A026-90BFD0CAF209@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8041503C-D087-434D-A026-90BFD0CAF209@vmware.com> ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="h6b35SB/"; spf=pass (imf04.hostedemail.com: domain of shiyn.lin@gmail.com designates 209.85.216.48 as permitted sender) smtp.mailfrom=shiyn.lin@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664305222; a=rsa-sha256; cv=none; b=Ki8DirK4JaGHsdMA1Db1DNbjmKeOwnB7MtsMVHFSeEkhZFQx74E4jv+lgmqZxUYfJN2/Ca mh59faH7nQMJ5UrBKA1oueXlJQD/Hm9XL5q5a8PD2k2/ixy20OmB/3SNwg6C03H/eQ8OrI MxPhjSG93/IGWDJHM9ajZKlg2GzL8+o= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664305222; 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=VPxbWHE5oCoBASI+IJV2mNevW1WyTdk4e8pg764ECb0=; b=gRnJ3qMj9bsg5Lj0Ye6s3i2Sv15elZAAZz5QcC2uhD7ln8Tn/nIdmEqXRZxhE+uEcln8s/ OHSIhVp4zndgZLBIhUItqH7JeIyKMcdXdCsz6JugrgK5ksR578JfoYZn5RHXich8V/NLyd +EsNuDigb0ExEgcYagA20ihRE4BjstY= Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="h6b35SB/"; spf=pass (imf04.hostedemail.com: domain of shiyn.lin@gmail.com designates 209.85.216.48 as permitted sender) smtp.mailfrom=shiyn.lin@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 7C59A40007 X-Stat-Signature: moyochrb93rkjsjdoo9wts4abyc6oo46 X-HE-Tag: 1664305222-103447 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 Tue, Sep 27, 2022 at 05:51:19PM +0000, Nadav Amit wrote: > > +static void cow_pte_rss(struct mm_struct *mm, struct vm_area_struct *vma, > > + pmd_t *pmdp, unsigned long addr, > > + unsigned long end, bool inc_dec) > > +{ > > + int rss[NR_MM_COUNTERS]; > > + spinlock_t *ptl; > > + pte_t *orig_ptep, *ptep; > > + struct page *page; > > + > > + init_rss_vec(rss); > > + > > + ptep = pte_offset_map_lock(mm, pmdp, addr, &ptl); > > + orig_ptep = ptep; > > + arch_enter_lazy_mmu_mode(); > > + do { > > + if (pte_none(*ptep)) > > + continue; > > + > > + page = vm_normal_page(vma, addr, *ptep); > > + if (page) { > > + if (inc_dec) > > + rss[mm_counter(page)]++; > > + else > > + rss[mm_counter(page)]--; > > + } > > + } while (ptep++, addr += PAGE_SIZE, addr != end); > > + arch_leave_lazy_mmu_mode(); > > + pte_unmap_unlock(orig_ptep, ptl); > > It seems to me very fragile to separate the accounting from the actual > operation. I do not see copying of the pages here, so why is the RSS > updated? Since it may have a situation like one process that doesn't do the accounting during COW, and it would want to reuse the table. So, we need to restore the states. On the other hand, it will have a situation like unmap the COWed table and wanting to remove the states. > > > + add_mm_rss_vec(mm, rss); > > +} > > + > > /* > > * This function is called to print an error when a bad pte > > * is found. For example, we might have a PFN-mapped pte in > > @@ -2817,6 +2848,68 @@ int apply_to_existing_page_range(struct mm_struct *mm, unsigned long addr, > > } > > EXPORT_SYMBOL_GPL(apply_to_existing_page_range); > > > > +/** > > + * cow_pte_fallback - reuse the shared PTE table > > + * @vma: vma that coever the shared PTE table > > + * @pmd: pmd index maps to the shared PTE table > > + * @addr: the address trigger the break COW, > > + * > > + * Reuse the shared (COW) PTE table when the refcount is equal to one. > > + * @addr needs to be in the range of the shared PTE table that @vma and > > + * @pmd mapped to it. > > + * > > + * COW PTE fallback to normal PTE: > > + * - two state here > > + * - After break child : [parent, rss=1, ref=1, write=NO , owner=parent] > > + * to [parent, rss=1, ref=1, write=YES, owner=NULL ] > > + * - After break parent: [child , rss=0, ref=1, write=NO , owner=NULL ] > > + * to [child , rss=1, ref=1, write=YES, owner=NULL ] > > + */ > > +void cow_pte_fallback(struct vm_area_struct *vma, pmd_t *pmd, > > + unsigned long addr) > > +{ > > + struct mm_struct *mm = vma->vm_mm; > > + struct vm_area_struct *prev = vma->vm_prev; > > + struct vm_area_struct *next = vma->vm_next; > > + unsigned long start, end; > > + pmd_t new; > > + > > + VM_WARN_ON(pmd_write(*pmd)); > > + > > + start = addr & PMD_MASK; > > + end = (addr + PMD_SIZE) & PMD_MASK; > > + > > + /* > > + * If pmd is not owner, it needs to increase the rss. > > + * Since only the owner has the RSS state for the COW PTE. > > + */ > > + if (!cow_pte_owner_is_same(pmd, pmd)) { > > + /* The part of address range is covered by previous. */ > > + if (start < vma->vm_start && prev && start < prev->vm_end) { > > + cow_pte_rss(mm, prev, pmd, > > + start, prev->vm_end, true /* inc */); > > + start = vma->vm_start; > > + } > > + /* The part of address range is covered by next. */ > > + if (end > vma->vm_end && next && end > next->vm_start) { > > + cow_pte_rss(mm, next, pmd, > > + next->vm_start, end, true /* inc */); > > + end = vma->vm_end; > > + } > > + cow_pte_rss(mm, vma, pmd, start, end, true /* inc */); > > + > > + mm_inc_nr_ptes(mm); > > + /* Memory barrier here is the same as pmd_install(). */ > > + smp_wmb(); > > + pmd_populate(mm, pmd, pmd_page(*pmd)); > > + } > > + > > + /* Reuse the pte page */ > > + set_cow_pte_owner(pmd, NULL); > > + new = pmd_mkwrite(*pmd); > > + set_pmd_at(mm, addr, pmd, new); > > +} > > Again, kind of hard to understand such a function without a context > (caller). For instance, is there any lock that prevents > cow_pte_owner_is_same() from racing with change of the owner? > It is called by the refcount operation and the break COW handler when the refcount is 1. Also, It uses synchronization primitives (in set_cow_pte_owner() and cow_pte_owner_is_same()) to prevent the race. > I would expect to see first patches that always copy the PTEs without > reusing the PTEs and only then a PTE reuse logic as an optimization. > I will restructure all the commits to make the logic clear. Thanks, Chih-En Lin