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 AA1C5C4345F for ; Tue, 16 Apr 2024 02:52:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 128E26B0087; Mon, 15 Apr 2024 22:52:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0B1176B0088; Mon, 15 Apr 2024 22:52:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E93396B0089; Mon, 15 Apr 2024 22:52:38 -0400 (EDT) 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 C77636B0087 for ; Mon, 15 Apr 2024 22:52:38 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 595D6808D1 for ; Tue, 16 Apr 2024 02:52:38 +0000 (UTC) X-FDA: 82013871996.09.69028B2 Received: from mail-vk1-f181.google.com (mail-vk1-f181.google.com [209.85.221.181]) by imf06.hostedemail.com (Postfix) with ESMTP id 8F2C9180004 for ; Tue, 16 Apr 2024 02:52:36 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BSk717Cr; spf=pass (imf06.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.181 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713235956; a=rsa-sha256; cv=none; b=esAZABw3SQvwwjmttQgAQXhfBUtyyFScpBHWXjtwFR7ILmTxAS1231Wt4GlIoEoQj09nyN wETYMRgl7nALypAbZfLMEKOFl7kSJmLcz049gxEKCS3P5hXKAE9f37OxO9SkQqgTbkXQMS a6jISNqZKeHlviiM2dZglUXCfK948bo= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BSk717Cr; spf=pass (imf06.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.181 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713235956; 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=lFczH1dIfyyTRf2W5n4H9U/PB8b6sWya9Y+4vbVP4sA=; b=JXxe6cR9+iPwjGaXYOmIvOXqIGBlf10AK91OrBAhT7FFR2bM+Fh0ybi5pxZqdQYakhQTh6 ajrW0Ms59PFshg+OODaudsJ8QTpN3YFgtjwQ9crBHedJl4nuQlB9OXVjbS44kWbqfKgwlR yN2AYHZkGBIWUdYB/fDKn1LSCmBJqv0= Received: by mail-vk1-f181.google.com with SMTP id 71dfb90a1353d-4daa91c0344so1814516e0c.3 for ; Mon, 15 Apr 2024 19:52:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713235956; x=1713840756; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=lFczH1dIfyyTRf2W5n4H9U/PB8b6sWya9Y+4vbVP4sA=; b=BSk717Crscv7NH+a7+Bqt9eVK+VorCuPVlzL0j6SK57iY1pMzPN7cE1Rw4ixaZcpcH Cv/DeTCp2Hfek4JpvFrP4zQ/zOpTda+kq7fS8XSATPFebQ0zyp8Vi190jP8dxIQkHFWi kS7ZpvbbPaLomVuEI/eqDl3zXp89WOe/rLY/8nM3s/3uSfZgJQAYxExW7HapfRU9QMYL 0L+SjHrvJ1MxOFX19GimeKupVu4/8mOmqwafHOTzKjnX7VIUxZAZVobhlZaQ3OD/jrbK /wxEtXIGQ3GopOZk8s1WyNJ3+rcRninGrmVlhPfsrZSv9f9xB95vSFc1e8iCle2A4uut cKQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713235956; x=1713840756; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lFczH1dIfyyTRf2W5n4H9U/PB8b6sWya9Y+4vbVP4sA=; b=sotRvWCErQz4hfnO8w6+x2y6I7502+5w8VVgNtOMVesYKL7iaH8BwGq8qlfCXPYCR/ luYNjlvVho9/9KZDCZJ60vCXV6YWj7nLrK11R/xBr8D8iIlVcqeaPE6Ln8/C3VEHJZ1F zxRiXmWMvMcAGe7FiHdfORJHh2ilHa5wxdbw/7oUgPMyvUiDumvHWsJaj401cZX1VhG5 TjfB9Jf13HXo9rDpCNPmlcRrSzE+fAQ3EyvWfXRk3+0WH8CL2y4xJXD0YtpddVFOoDod 8rqQyT2buL7hKclrurWm43CpZKATurUxqXkMEpV88ACA+iD8P7mL5PwGH06lt7lDM24w fIfw== X-Forwarded-Encrypted: i=1; AJvYcCUP0CSUxW7s4s2W5eDSGPoewVyxzzReJN0x6DypJAHfiBQQOv305oiK3YhFTuHywybJ5JqMpjp56Zh47s3JhfzhsNQ= X-Gm-Message-State: AOJu0YwIOBP0lGe4VHG4RkQ7YyaHRRS4UELaboVBwlmUt0uRDbWCEo4N jvhQUZIvBQ6YyRNLtyoXh5JzWzs2C7v1N95pyJuz7hfTWocONbmKy0m4zq50ZxVb1TTo0DAUjwd ipUeCACoyDeBGkFVAbfFtzkNG8RA= X-Google-Smtp-Source: AGHT+IHMrnSUhuVEuo4y3tiKOPKziGLcQQRIaE2TzTqfVCk8KiKNdVf22ivrucySUANuiuxuBay5T1zPpM1Wy/chv9Q= X-Received: by 2002:a05:6122:3d14:b0:4d4:ef9:719d with SMTP id ga20-20020a0561223d1400b004d40ef9719dmr9351058vkb.5.1713235955700; Mon, 15 Apr 2024 19:52:35 -0700 (PDT) MIME-Version: 1.0 References: <20240409082631.187483-1-21cnbao@gmail.com> <20240409082631.187483-5-21cnbao@gmail.com> <87frvn2f89.fsf@yhuang6-desk2.ccr.corp.intel.com> <8734rm2gdj.fsf@yhuang6-desk2.ccr.corp.intel.com> <87y19e115l.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87y19e115l.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Barry Song <21cnbao@gmail.com> Date: Tue, 16 Apr 2024 14:52:24 +1200 Message-ID: Subject: Re: [PATCH v2 4/5] mm: swap: entirely map large folios found in swapcache To: "Huang, Ying" Cc: Khalid Aziz , akpm@linux-foundation.org, linux-mm@kvack.org, baolin.wang@linux.alibaba.com, chrisl@kernel.org, david@redhat.com, hanchuanhua@oppo.com, hannes@cmpxchg.org, hughd@google.com, kasong@tencent.com, ryan.roberts@arm.com, surenb@google.com, v-songbaohua@oppo.com, willy@infradead.org, xiang@kernel.org, yosryahmed@google.com, yuzhao@google.com, ziy@nvidia.com, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 8F2C9180004 X-Stat-Signature: t37yksoiyf8em8kpczjpchrqt93hx9wk X-Rspam-User: X-HE-Tag: 1713235956-65761 X-HE-Meta: U2FsdGVkX18r1IVKznSn56Yjx+EkYLgJE0lqDf2DbXt7+rjKzxtmAUtcTAV3t7k3sE+WP83FXV85DLRfoD9yTrHUEoQLqlIlkN5NKFY70SKBNOqCtrzLDMcWadbv+6JfTQqefqWI0kIEbIr3j+7O3KpZmoU4Q6uT3PzfIroxUWe1iGOHCdjhMDXrLylC+FYtUA5Fa3H44BEgwVqPuymOo5c4QqPD8049ucmGmoJTICn1LDPuAm91b/j6Kl5aMaJYgG8yffkm1zXVg7zLypOrvMs8mLWpjSmy52OW3R0V22QYFOU+1ffrj7mWuxYAAE2DiJe34cft++t04tTigN1kYidhI1JbnXPbVLlPCPZZICtjB/uGhScOEdQ1lEz1tEqDtZZfwSbz55FOb9A9G5X0JmzGeaXSrHHnNXHwwR8Mz4LRxUr1oJrqb1sbMvWR3ZANFwQVhPdmHTPaDkgUohsK/+G/a5X7wBH9Em1wAGD6GJLxcgS3R6o7rxSW7ZBF4WFwDqXZNNt3znVrxEvlWzHU/V7Cr5h8d+xGSMCuqUvenPimF70dYsRclkd+eUmqG0uqf4W+0ppJ4Oe2Dbb2xkpKVujKei3NMF7vSoCER5VIEsMp/zOCVS39+Vw9m2IwZr4bOUD+QH1SfZYI5OWz/zqAkl2vHz+J1aDQn+xOWj08of38S7XE7eGvUfScVvoKUo4mk5Qs89rN4aPiKjzkWEzWECVl6CzjAfQMtiG/ZgLYWW21C3MAOQZ4l/X1UOcD9BpWvKsxRMgLsACB0Frr25T4gEcSW4sWe9ORqvVjXG21Ba/5ie+y4DsbHzauZDVTCszeFPMRQKDLmH8qo46zmsBVD40jtXi6dyb8eCAmkaE2yWZmGJ8Ir3trHVci/54SZa/S320rqHxPF/xIWCDlWQvpPPUmq7zdblMTDTl/ZRMi0i9oB+bEhb/zIlaYxzSpfxwFICJX6GRxJwJLat4NF2R TUqQVyjJ WgwFGtp5zF4uelouNMcRMBpK3N7VUeG3E/qSB0c0ZjAF9395RaV8tMXzZ7CpTJ0nusbTTwGyMNULQDrUdlY3NZnxD37u5xEPOYJ9Cu9+KXirinXWdVyuShWUY3G2z1PqXgt8/1HQAeUBj5ZiMjrEzZa5Ee6P3OQGesW+6hMLRl2Gid6PsQ7Zt1D4XrcdjHzxlE+RZP+4LorreioWwOhquI7hJJdobiMY9kliGWkn65bM75kak+a7ZORhlrxHKEe9O6F0k3oh5NIX+KM/Bg58RO9GmsW68OpuTBsTL+Gc1HfiLtQYYLjF3+2boGr0lzTOgEV6InFNsOG6FNYTtPCYxrnaNS+JJBe22kwkzSgxHvy31jyhunTwRFXjxUUke3Zpee5Vml1yUq2LWfq8= 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 Tue, Apr 16, 2024 at 2:41=E2=80=AFPM Huang, Ying = wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > On Tue, Apr 16, 2024 at 2:27=E2=80=AFPM Huang, Ying wrote: > >> > >> > >> Added Khalid for arch_do_swap_page(). > >> > >> Barry Song <21cnbao@gmail.com> writes: > >> > >> > On Mon, Apr 15, 2024 at 8:39=E2=80=AFPM Huang, Ying wrote: > >> >> > >> >> Barry Song <21cnbao@gmail.com> writes: > >> > >> [snip] > >> > >> >> > >> >> > + bool any_swap_shared =3D false; > >> >> > > >> >> > if (!pte_unmap_same(vmf)) > >> >> > goto out; > >> >> > @@ -4137,6 +4141,35 @@ vm_fault_t do_swap_page(struct vm_fault *v= mf) > >> >> > */ > >> >> > vmf->pte =3D pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf-= >address, > >> >> > &vmf->ptl); > >> >> > >> >> We should move pte check here. That is, > >> >> > >> >> if (unlikely(!vmf->pte || !pte_same(ptep_get(vmf->pte), vmf= ->orig_pte))) > >> >> goto out_nomap; > >> >> > >> >> This will simplify the situation for large folio. > >> > > >> > the plan is moving the whole code block > >> > > >> > if (start_pte && folio_test_large(folio) && folio_test_swapcache(fol= io)) > >> > > >> > after > >> > if (unlikely(!folio_test_uptodate(folio))) { > >> > ret =3D VM_FAULT_SIGBUS; > >> > goto out_nomap; > >> > } > >> > > >> > though we couldn't be !folio_test_uptodate(folio)) for hitting > >> > swapcache but it seems > >> > logically better for future use. > >> > >> LGTM, Thanks! > >> > >> >> > >> >> > + > >> >> > + /* We hit large folios in swapcache */ > >> >> > >> >> The comments seems unnecessary because the code tells that already. > >> >> > >> >> > + if (start_pte && folio_test_large(folio) && folio_test_swap= cache(folio)) { > >> >> > + int nr =3D folio_nr_pages(folio); > >> >> > + int idx =3D folio_page_idx(folio, page); > >> >> > + unsigned long folio_start =3D vmf->address - idx * = PAGE_SIZE; > >> >> > + unsigned long folio_end =3D folio_start + nr * PAGE= _SIZE; > >> >> > + pte_t *folio_ptep; > >> >> > + pte_t folio_pte; > >> >> > + > >> >> > + if (unlikely(folio_start < max(vmf->address & PMD_M= ASK, vma->vm_start))) > >> >> > + goto check_pte; > >> >> > + if (unlikely(folio_end > pmd_addr_end(vmf->address,= vma->vm_end))) > >> >> > + goto check_pte; > >> >> > + > >> >> > + folio_ptep =3D vmf->pte - idx; > >> >> > + folio_pte =3D ptep_get(folio_ptep); > >> >> > >> >> It's better to construct pte based on fault PTE via generalizing > >> >> pte_next_swp_offset() (may be pte_move_swp_offset()). Then we can = find > >> >> inconsistent PTEs quicker. > >> > > >> > it seems your point is getting the pte of page0 by pte_next_swp_offs= et() > >> > unfortunately pte_next_swp_offset can't go back. on the other hand, > >> > we have to check the real pte value of the 0nd entry right now becau= se > >> > swap_pte_batch() only really reads pte from the 1st entry. it assume= s > >> > pte argument is the real value for the 0nd pte entry. > >> > > >> > static inline int swap_pte_batch(pte_t *start_ptep, int max_nr, pte_= t pte) > >> > { > >> > pte_t expected_pte =3D pte_next_swp_offset(pte); > >> > const pte_t *end_ptep =3D start_ptep + max_nr; > >> > pte_t *ptep =3D start_ptep + 1; > >> > > >> > VM_WARN_ON(max_nr < 1); > >> > VM_WARN_ON(!is_swap_pte(pte)); > >> > VM_WARN_ON(non_swap_entry(pte_to_swp_entry(pte))); > >> > > >> > while (ptep < end_ptep) { > >> > pte =3D ptep_get(ptep); > >> > > >> > if (!pte_same(pte, expected_pte)) > >> > break; > >> > > >> > expected_pte =3D pte_next_swp_offset(expected_pte); > >> > ptep++; > >> > } > >> > > >> > return ptep - start_ptep; > >> > } > >> > >> Yes. You are right. > >> > >> But we may check whether the pte of page0 is same as "vmf->orig_pte - > >> folio_page_idx()" (fake code). > > > > right, that is why we are reading and checking PTE0 before calling > > swap_pte_batch() > > right now. > > > > folio_ptep =3D vmf->pte - idx; > > folio_pte =3D ptep_get(folio_ptep); > > if (!is_swap_pte(folio_pte) || non_swap_entry(pte_to_swp_entry(folio_= pte)) || > > swap_pte_batch(folio_ptep, nr, folio_pte, &any_swap_shared) !=3D = nr) > > goto check_pte; > > > > So, if I understand correctly, you're proposing that we should directly= check > > PTE0 in swap_pte_batch(). Personally, I don't have any objections to th= is idea. > > However, I'd also like to hear the feedback from Ryan and David :-) > > I mean that we can replace > > !is_swap_pte(folio_pte) || non_swap_entry(pte_to_swp_entry(folio_= pte)) > > in above code with pte_same() with constructed expected first pte. Got it. It could be quite tricky, especially with considerations like pte_swp_soft_dirty, pte_swp_exclusive, and pte_swp_uffd_wp. We might require a helper function similar to pte_next_swp_offset() but capable of moving both forward and backward. For instance: pte_move_swp_offset(pte_t pte, long delta) pte_next_swp_offset can insteadly call it by: pte_move_swp_offset(pte, 1); Is it what you are proposing? > > >> > >> You need to check the pte of page 0 anyway. > >> > >> >> > >> >> > + if (!is_swap_pte(folio_pte) || non_swap_entry(pte_t= o_swp_entry(folio_pte)) || > >> >> > + swap_pte_batch(folio_ptep, nr, folio_pte, &any_= swap_shared) !=3D nr) > >> >> > + goto check_pte; > >> >> > + > >> >> > + start_address =3D folio_start; > >> >> > + start_pte =3D folio_ptep; > >> >> > + nr_pages =3D nr; > >> >> > + entry =3D folio->swap; > >> >> > + page =3D &folio->page; > >> >> > + } > >> >> > + > >> >> > +check_pte: > >> >> > if (unlikely(!vmf->pte || !pte_same(ptep_get(vmf->pte), vmf= ->orig_pte))) > >> >> > goto out_nomap; > >> >> > > >> >> > @@ -4190,6 +4223,10 @@ vm_fault_t do_swap_page(struct vm_fault *v= mf) > >> >> > */ > >> >> > exclusive =3D false; > >> >> > } > >> >> > + > >> >> > + /* Reuse the whole large folio iff all entries are = exclusive */ > >> >> > + if (nr_pages > 1 && any_swap_shared) > >> >> > + exclusive =3D false; > >> >> > } > >> >> > > > [snip] > > -- > Best Regards, > Huang, Ying