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 307E8C4345F for ; Tue, 16 Apr 2024 04:40:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 860086B0085; Tue, 16 Apr 2024 00:40:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 80F1B6B0087; Tue, 16 Apr 2024 00:40:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6B91B6B0088; Tue, 16 Apr 2024 00:40:22 -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 4CE636B0085 for ; Tue, 16 Apr 2024 00:40:22 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 026AF1208C1 for ; Tue, 16 Apr 2024 04:40:21 +0000 (UTC) X-FDA: 82014143484.08.1AA4F24 Received: from mail-vk1-f182.google.com (mail-vk1-f182.google.com [209.85.221.182]) by imf27.hostedemail.com (Postfix) with ESMTP id 3A40D4000C for ; Tue, 16 Apr 2024 04:40:20 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PFxdpgCY; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf27.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.182 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713242420; 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=6DNUNm1nTQTzvmw64PZzPb0ANLIFLIl6nmYK+uOcBCY=; b=B0gA46y2nqBw9ZhCuMuUhfxYEWbxAxSUz0fkwhNqdtsETK3I0fnRUso7VL46mDUYKI5y74 yrCvOo0XW9tI6oBo8o5HDAJ4zcFcVhObZ2dYR7yOCj4+ejquFdxXo4jWu0TS2IvvHs+7/S zoLV9yUT7QiNu8ZYLFY8/7H0Y/at1tY= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PFxdpgCY; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf27.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.182 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713242420; a=rsa-sha256; cv=none; b=xSXrMMmZ8ouLpfUEvh8a7LvtG4cZ5k4X+pusfp/FALZ0L/td148nGfk8J4AJbv7B4EdeOw vjoVaMlDVzPrJN2Urj0lgpWCbYIIzCRiUNkwuhLlLs35RuNpIrxf6MbW+2UPj+f7aztM2/ SekkARxTI6To6GNHlTRUqQA+fwh8s/w= Received: by mail-vk1-f182.google.com with SMTP id 71dfb90a1353d-4daab3cf090so2142501e0c.0 for ; Mon, 15 Apr 2024 21:40:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713242419; x=1713847219; 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=6DNUNm1nTQTzvmw64PZzPb0ANLIFLIl6nmYK+uOcBCY=; b=PFxdpgCY33AGkH6wQAoveL6T9PqgmDjvIhlAN4gytaI7pE8YzRZWYQUskLfdxviXT0 fuuZKnjbfcE7VoR82jhFgWaktH/9lSpchvCmqYlMMeZNWAQE89ffQx5ZGjE26QSIjgZy VPLlvoJujA55VULb2ArENyzDcxnXX97QInO/vXbq8jOSl3seIIV95szC0iE0EOm1l/T3 AF/r5RLQodNEUtfOgxMW8mGp3dH5aFoqsd3gvEREKa0DRCjZov/WQHhQO1i5xXj2StGX 17xDJcec4eYNl2LznOCTTxASWkb/hPGO2v0UcRPjfj44CfA50pjryxeRT8uUFCBhQ4Sz 9geA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713242419; x=1713847219; 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=6DNUNm1nTQTzvmw64PZzPb0ANLIFLIl6nmYK+uOcBCY=; b=BiwqnhX76dxvyvZRORdviw15Qz9kxwBWtmiDVOmgplbipew0Fs2qXhTrR7QkAvAHEA Gp6rNzLTmNXREHJsV/6GqXGbBGzEoTNKWNQl/25Ovi7NiGDwwhS9PMskDdoz8PGfUXhK vT4Pc9tLBBjNq+iU2BJRsAM7jRCddxtcNSbI+aq8wV/j4NeqdoBb3zf7iLe+ov7Po4M7 vMJNIINyMGsu91ka+5DYAWcvwWYjHpBw66m/d5U0Urxsge7VkewjSAiv3o78VY660xV0 lASpklndQkx/6whnKCpSWO+PwMIHIFafg1J8I9gAxXQmi/tRbaKa/YPDl8NbIG2a+/X9 Hfhw== X-Forwarded-Encrypted: i=1; AJvYcCWssW0ujX9gLNn/WCvpRsTt1JFbO8g2i5r9eyEsIud8MLKVaHRKvG734YeXQ1AB7LvF4A5zon0d3mb0jKvqIymaQD0= X-Gm-Message-State: AOJu0YxNMC977DyEM1OF3QPHhx2xXI7pGsQK60NF6ARC1U3afXjbmCfV o8f/sgtRu/9AsNl+WserFNkxTzwG+NURdWz4uV71mkg8zpii/ElWcH+hfymjcPnvcbPPaCt+Sau 5GyhLIp35eEhcNlBEMusMrL45eLc= X-Google-Smtp-Source: AGHT+IFYyNyG43lazOLMtibqNC/p6+JtCDmf8y2Uyn7ZB9V4TTE49O/qma2mk0uajdyxjsyuCxHN4gT4dtoFPo7T9oI= X-Received: by 2002:a05:6122:c84:b0:4d3:34f4:7e99 with SMTP id ba4-20020a0561220c8400b004d334f47e99mr590473vkb.0.1713242419205; Mon, 15 Apr 2024 21:40:19 -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> <87pluq0zdr.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87pluq0zdr.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Barry Song <21cnbao@gmail.com> Date: Tue, 16 Apr 2024 16:40:08 +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-Rspam-User: X-Stat-Signature: i9mge3puspeau3da3py1qea9e3hjp1ms X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 3A40D4000C X-HE-Tag: 1713242420-570094 X-HE-Meta: U2FsdGVkX1/+K3ddEHIUbOYam87go4eiShNeN5f0aWUAx0+06xVfLA7ZemGrcO9pxFROsjm2zsaB7PJ+Ypfk+uQekLJdXxQT3kHG4GoiEflfNMgnjlHqC+59rlVN0/OniGtgXFYqHtjYU8h59TduIPqONuQJNNVhyd+blsXSTor0JIb8rsbLHy/bpP/K5R2gY2Rch5l7vY64re/ahe850JwfYJ7Ovvh6EFvDVrkNoiM2lcoEcxkdnCv5oZ8xdtWs/iigYzhHDLKiMX5BajvK/VuJaRdLauBVcW+LH+bvJa/uMJhjqWcxlElDqrf/JLfk/ikOvl27MHTUPjnzDWqEz3tDM8ZSsk8q5miyNb3OTkU1DlBj32YBS5sJxY+/+ThvUs9Oz2PZhPlxZCfG4BdGfQ3fRdkVD9q1abJz+fseGjkhJwWf2DgiVn2folPl4qmz7cdrN4DIJmKqS3OQe+4kCo5ccLcWMwAl/luHRcWcRGkS5Ous3GvPEczDVexK0gy9fq83XQPMVEYXB/QlAorhAiXCXK95jCmKm8KozCovnWgMYQr6k+AxVcncYlF9YwMm6hN+pJ1bMvWuXxCgDvgJ8wxCGzF2ae1004ArIovQTFtgtXMZ0Vb2TZlMegB3knTZziENZGkhUL4EggcP87/alYUBfNrW3EXcAQMBZEnz74LNL65HOran2yh8+F2uG5n4EIx2UluMGdruTBXYlL3vUk2Z1aJTk24TiW66S5BZXpd6xgNZgb/yb8xa24wlIq8hrgiyDhtSwTbgRv3ZwrWx7b/sJQ0bFL3GwT+tgqqwDt/6FvoIgzSEpCw3nPTfTYMvCqU2x15tu4sGU2xCuoNVvm6jCSMlbMwyIheVqe9uTrhy1dPk6e7WGzImjxG82rh7AUsi4EgCKo6hjWJhDCd1UBlthNbErcD5n34ljSEfuiChJqczDYBtWXYMhEEFq1gT8PIUXO1lArmYsOBjom7 ysysZgFl mjUeEwdbwIBHySgv1eiL8Rdus/zG+joxHvT4KazLL3xzBbrV4bDZ+6hevTtOqZeZTlLCDSij7y8BZM7PuZAre+vDJ3yzktIP1JM82O9tPeG1NFmWZJ127XJclXOatUIqDI7G19LE8MvM5b/+FJOD8q+E1UqrBD3D0TeEER3jmIy3bUTtHH402YEdIkiTRDZJhj59K1u/VUQLfbOimmyxi1lnNHsIvqsyNKbpPdnDsmyijTccMwYF9S/XQQFzRd+dhngIP2fnrqAi4/9QV2UStlqvuJ+HB7aMJvpgCLZUR4P6X6/LZq1tMQzYC8SneTLyW+1jnam41Rd6gz82bGeOq7Bb8ShSmphftCpiQKfhk2Fl1e6HMI8ZnyNQoAwJTiBauAiP6mSblmKimpDlHQ+Kia1AGcA== 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 3:19=E2=80=AFPM Huang, Ying = wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > 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= *vmf) > >> >> >> > */ > >> >> >> > vmf->pte =3D pte_offset_map_lock(vma->vm_mm, vmf->pmd, v= mf->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(= folio)) > >> >> > > >> >> > 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 alrea= dy. > >> >> >> > >> >> >> > + if (start_pte && folio_test_large(folio) && folio_test_s= wapcache(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 * P= AGE_SIZE; > >> >> >> > + pte_t *folio_ptep; > >> >> >> > + pte_t folio_pte; > >> >> >> > + > >> >> >> > + if (unlikely(folio_start < max(vmf->address & PM= D_MASK, vma->vm_start))) > >> >> >> > + goto check_pte; > >> >> >> > + if (unlikely(folio_end > pmd_addr_end(vmf->addre= ss, 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 c= an find > >> >> >> inconsistent PTEs quicker. > >> >> > > >> >> > it seems your point is getting the pte of page0 by pte_next_swp_o= ffset() > >> >> > unfortunately pte_next_swp_offset can't go back. on the other han= d, > >> >> > we have to check the real pte value of the 0nd entry right now be= cause > >> >> > swap_pte_batch() only really reads pte from the 1st entry. it ass= umes > >> >> > pte argument is the real value for the 0nd pte entry. > >> >> > > >> >> > static inline int swap_pte_batch(pte_t *start_ptep, int max_nr, p= te_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(fol= io_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 direc= tly check > >> > PTE0 in swap_pte_batch(). Personally, I don't have any objections to= this 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(fol= io_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? > > Yes. Exactly. Great. I agree that this appears to be much cleaner than the current code. > > -- > Best Regards, > Huang, Ying > > >> > >> >> > >> >> You need to check the pte of page 0 anyway. > >> >> > >> >> >> > >> >> >> > + if (!is_swap_pte(folio_pte) || non_swap_entry(pt= e_to_swp_entry(folio_pte)) || > >> >> >> > + swap_pte_batch(folio_ptep, nr, folio_pte, &a= ny_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= *vmf) > >> >> >> > */ > >> >> >> > exclusive =3D false; > >> >> >> > } > >> >> >> > + > >> >> >> > + /* Reuse the whole large folio iff all entries a= re exclusive */ > >> >> >> > + if (nr_pages > 1 && any_swap_shared) > >> >> >> > + exclusive =3D false; > >> >> >> > } > >> >> >> > > >> > >> [snip] > >> > >> -- > >> Best Regards, > >> Huang, Ying