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 3B1DCC4332F for ; Sun, 11 Dec 2022 11:48:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 849108E0003; Sun, 11 Dec 2022 06:47:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7F88D8E0001; Sun, 11 Dec 2022 06:47:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 699D18E0003; Sun, 11 Dec 2022 06:47:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 575688E0001 for ; Sun, 11 Dec 2022 06:47:59 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 22A2D1208B6 for ; Sun, 11 Dec 2022 11:47:59 +0000 (UTC) X-FDA: 80229851478.08.B7EB77E Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) by imf30.hostedemail.com (Postfix) with ESMTP id 660868000F for ; Sun, 11 Dec 2022 11:47:57 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="n/JD7w3x"; spf=pass (imf30.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.171 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670759277; a=rsa-sha256; cv=none; b=h5gQUqV0sPZXstdlQcMdGNT0EFjC+jEq+i2OTJxXp/Jk4zSb16WhyhG736vYYFcTaMDRMF JIXCA+H2aK/kNPNiKo+t97Sp9MamPCp+p8PcKbA+VBan68/F0rp04hzgW0+B/8ns5dVXXV rvjcW4Miu4ZeEfvYb+vXTg7TZToy+Bk= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="n/JD7w3x"; spf=pass (imf30.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.171 as permitted sender) smtp.mailfrom=ryncsn@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=1670759277; 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=BOuPxWYpFP09QQXPfRxhnJ/8wDMydFyrgaAcKSHPSrA=; b=v2AazENXxJEPHMLqjopeGn4qTWnrEgosjMJBD0ELLPo5RbdpYm/JiqMm7G9wRe68QpqBDw jcIJWh4isd2tI9OM2dQJ57uvq/3WhCVKyIqUvnyqZl6xMz9w/LZ05/JUkK/TFgWFxFAEiG 5Lmx+7lwnDVwbB+NoNqbZn+P7dHCXgE= Received: by mail-lj1-f171.google.com with SMTP id z4so9648956ljq.6 for ; Sun, 11 Dec 2022 03:47:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=BOuPxWYpFP09QQXPfRxhnJ/8wDMydFyrgaAcKSHPSrA=; b=n/JD7w3xfDrZ+55g6bXLtgigY98gi4rX/mB6VUaC/UIjM3NFDDePEYkDHcpSJfxEn8 wRHiS/P1PEPJHq2aXdz62uSX1995X13pu/V2KUD0gvy9Kb9R9+bpa9Oik0eREuQsiSCD JP+aq192CPrqIwFQ2Qs34ptQqdGkt5NHEhiV24vbWK46WWszfAT1A8SExueiKej3MDiH uoiqzLMUiWd6S1VpxYBCMJ/9Wrbh2s+RL6eui0799WQ9J7Vd4kR4T+fd1YSMs3s8Fe7Y p5hOwbXQHCExLwZy9psdhVwRwiHjmp4GaV8C6ROlQnc9J38HYdFvHkH8qWcDAkkY+/CE IOqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=BOuPxWYpFP09QQXPfRxhnJ/8wDMydFyrgaAcKSHPSrA=; b=xZHnQZFah/c3fnrql+jO8rYcOp5BRDLPFyufeRq56Am3nraPmgH824LivTResUsPaq HtixkbfDByUCl2J4h0nUmiV4tFQRc5Tw+oU83prLpmv1x/qF9AyTrX4iG/t0+hLtIbXt Kr/6NfZsiR+yM5NolPoROnVVbhB7YEOGR3fSteghwGl5cYN0lLFmFoBDhETAdloa3OCj i9+d3CyxiMGvIFF8W+1s3oCJKd8L7Lw/GZmONJ4JQXvLoYuVEYEsiMdicOwXOLPb6jpT D9o4YNcqfps7MNIJ05pGUCoOaIvEysgkWWOA5JjZdUBFf+AU4F1BLjQW4ubazrmcRlEs HnHA== X-Gm-Message-State: ANoB5pl6Ub5hHXNYXN3M0T13bdgXYZimIaUkwqbYUN2if7dw4kAVkaKz Yz/oSeGKB7balBCTgldBIgYqfIjnByTNwEibYDc= X-Google-Smtp-Source: AA0mqf7wXjpazljqZ9v0DJ/NKU49RGa0aWZcce2hL2IwPTKUpn7Z5/3E+Tdkxc8A3CFSne7Ef7sb4tA+mauHypAGJQQ= X-Received: by 2002:a2e:83c5:0:b0:277:aed:be6b with SMTP id s5-20020a2e83c5000000b002770aedbe6bmr27163729ljh.322.1670759275526; Sun, 11 Dec 2022 03:47:55 -0800 (PST) MIME-Version: 1.0 References: <20221208180209.50845-1-ryncsn@gmail.com> <20221208180209.50845-5-ryncsn@gmail.com> <87iliiqidl.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87iliiqidl.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Kairui Song Date: Sun, 11 Dec 2022 19:47:43 +0800 Message-ID: Subject: Re: [PATCH 4/5] swap: remove the swap lock in swap_cache_get_folio To: "Huang, Ying" Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Miaohe Lin , David Hildenbrand , Hugh Dickins Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 660868000F X-Stat-Signature: 3ngxc6d3k7cthc4wybcy4exoc9tn6yks X-HE-Tag: 1670759277-926893 X-HE-Meta: U2FsdGVkX19OgUqAQlLhqc7GH9Le/qoLijQWdfu24ALL0cftweX9paN/KIvQnDsJ11iwykIPkzGqLCIBS6kh7ULA7ZZ8vWks6+LAd397vFunPtkXCbKEQUPcXA3OZIJvDf2pOmClCTw10FSm/q6wU5p9ktTvPxm6wtZJCS2/YmfMpsqSMb34OIV5zVPtbX7qlkX+ngY06rd4LQVsKboxNkS19c/RBIRmdDcZ0J7mQ7yoLjUQf3SlZ/N6RQefSfXYAj4raxRpst5LDXGL0R2a0B9Cv9+oI4YxB7nhAycuwCpGFg+tVGpc1J37Fuvps/K6gvYOenllvTMr7qOAMB5UOwiNDy3MvMupQX400SMc1SqnjcbnON9dd/ohoWDLfcDA+zEFK0QxNVHQXww0pWNqpu7hhUPsjY+kPtmS6AlXnB4/mibcVoWllQN+2EheehN9Y+2WT/trCVyefKRSuXb9+3n2Nd/IHA0+Eocx1fq77D+5tbDitU3lHlIQM45lFqT5JxxGwBC29Nya+5+UP3YBvM2bElufMN+gqYbjLBfemTZVpofzbbmVa7N05VPu5NJ+2G8LDbv1fHVLW1KpSEExlJVTKJ4SjSjRlHt4yqPb6P76+/F4byuKZxMUBrVyzlWrd7NvPjeBz03PJLEmLtbCSx7lZDSTJlP8XJ4JEWHEjRftGu0Vasph18+3NIcfLIbO+HQVo9RfYu2Qcb1Z9YpwzSyMBzkKnhvsR0sn+BIn5s66mi8kzkCacoG83swxP403zz0QcrpVWJkk7CQD5MW8b02Kd1+6V+NRyzMwEe174/j7XIzwdsHaEl1ra552eWxSD20Hg/JMeh76BS2NQbI7CSvqqnD1aVakgPuIWAzxBqb22ntEoqjqxiJ0HbdAsAonkA/UVKbzZkBUENCuv2JeSdTlUNbFsNB8W4nbiclogM2OSiOyKrRJFgcLGMj5NI+uuG0c+T2yDPeh5+NyA2+ DQUvckBV WNrdw5zS89TOd34bjfTSSrPydw0KVJJJgpytQr4XlDkjtgDbwXtlJt9l10nKQXjX/R0pJntb9OR6MN4q8/Qj0u/bkEIZDtr3WeQsm 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: Huang, Ying =E4=BA=8E2022=E5=B9=B412=E6=9C=8811=E6= =97=A5=E5=91=A8=E6=97=A5 19:40=E5=86=99=E9=81=93=EF=BC=9A > > Kairui Song writes: > > > From: Kairui Song > > > > There is only one caller not keep holding a reference or lock the > > swap device while calling this function. Just move the lock out > > of this function, it only used to prevent swapoff, and this helper > > function is very short so there is no performance regression > > issue. Help saves a few cycles. > > > Subject: Re: [PATCH 4/5] swap: remove the swap lock in swap_cache_get_f= olio > > I don't think you remove `swap lock` in swap_cache_get_folio(). Just > avoid to inc/dec the reference count. Yes, that's more accurate, it's kind of like 'locked the swap device from being swapped off', so I used some inaccurate word. I'll correct this in V2. > > And I think it's better to add '()' after swap_cache_get_folio to make > it clear it's a function. Good suggestion. > > > Signed-off-by: Kairui Song > > --- > > mm/shmem.c | 8 +++++++- > > mm/swap_state.c | 8 ++------ > > 2 files changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > index c1d8b8a1aa3b..0183b6678270 100644 > > --- a/mm/shmem.c > > +++ b/mm/shmem.c > > @@ -1725,6 +1725,7 @@ static int shmem_swapin_folio(struct inode *inode= , pgoff_t index, > > struct address_space *mapping =3D inode->i_mapping; > > struct shmem_inode_info *info =3D SHMEM_I(inode); > > struct mm_struct *charge_mm =3D vma ? vma->vm_mm : NULL; > > + struct swap_info_struct *si; > > struct folio *folio =3D NULL; > > swp_entry_t swap; > > int error; > > @@ -1737,7 +1738,12 @@ static int shmem_swapin_folio(struct inode *inod= e, pgoff_t index, > > return -EIO; > > > > /* Look it up and read it in.. */ > > - folio =3D swap_cache_get_folio(swap, NULL, 0); > > + si =3D get_swap_device(swap); > > + if (si) { > > + folio =3D swap_cache_get_folio(swap, NULL, 0); > > + put_swap_device(si); > > I'd rather to call put_swap_device() at the end of function. That is, > whenever we get a swap entry without proper lock/reference to prevent > swapoff, we should call get_swap_device() to check its validity and > prevent the swap device from swapoff. Yes, that's the right way to do it, my code is buggy here, sorry for being so careless, I'll fix it. > > Best Regards, > Huang, Ying > > > + } > > + > > if (!folio) { > > /* Or update major stats only when swapin succeeds?? */ > > if (fault_type) { > > diff --git a/mm/swap_state.c b/mm/swap_state.c > > index 19089417abd1..eba388f67741 100644 > > --- a/mm/swap_state.c > > +++ b/mm/swap_state.c > > @@ -324,19 +324,15 @@ static inline bool swap_use_vma_readahead(void) > > * unlocked and with its refcount incremented - we rely on the kernel > > * lock getting page table operations atomic even if we drop the folio > > * lock before returning. > > + * > > + * Caller must lock the swap device or hold a reference to keep it val= id. > > */ > > struct folio *swap_cache_get_folio(swp_entry_t entry, > > struct vm_area_struct *vma, unsigned long addr) > > { > > struct folio *folio; > > - struct swap_info_struct *si; > > > > - si =3D get_swap_device(entry); > > - if (!si) > > - return NULL; > > folio =3D filemap_get_folio(swap_address_space(entry), swp_offset= (entry)); > > - put_swap_device(si); > > - > > if (folio) { > > bool vma_ra =3D swap_use_vma_readahead(); > > bool readahead;