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 3A465C001B2 for ; Sun, 11 Dec 2022 11:40:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3E1198E0003; Sun, 11 Dec 2022 06:40:39 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 390D08E0001; Sun, 11 Dec 2022 06:40:39 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 258938E0003; Sun, 11 Dec 2022 06:40:39 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 132288E0001 for ; Sun, 11 Dec 2022 06:40:39 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id D01D51607AB for ; Sun, 11 Dec 2022 11:40:38 +0000 (UTC) X-FDA: 80229832956.07.B1C74FA Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by imf29.hostedemail.com (Postfix) with ESMTP id 2796F120006 for ; Sun, 11 Dec 2022 11:40:35 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=K27QzmDr; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf29.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.120 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670758837; 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=Kc+X9z1hnZSuyfbooDc5hc0JtYu5/h+ze810jUro6BA=; b=laVrHsCZXB+pJMwCb2g/io4bz5/4+tV4qNqQy9j1UlUAVCwaVOxZH88238QhbXt9pDQmsT K9gSwclTz7MeLSlRezAQCuZLyeSuTjqVWPwrSMxpCH9Y+cSzVcpwEXQySXh94eB/qsRvWC b3tZdXqjZsat4E9jMFQ4K8lySgAVD1M= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=K27QzmDr; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf29.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.120 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670758837; a=rsa-sha256; cv=none; b=wMpR5Hezqg0CD65Jp9fMYK/BjNVFE5uaPOeFSpV8vwAcZaBQqIcu69ZD82UtOWGn4IByRQ 0asjMkqXhsz8Jg+6kXfl+1++Jl80DO97+D5km+WHXY2WwoXoKzXkdkZjfwEAvqt5qMapTI A1O8E+xO2UzSGSEinlrQMvv1Ru+HkPU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1670758836; x=1702294836; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=astvZh8nWsVzPt1jQxCFv8l0mlIeuLksFqxluzjYH1k=; b=K27QzmDr2mkJOaje2tJUW1rMSeHu043Pbc/7k52GobfL2Sv+cAIBaJmA Tw19vywZ08/OssaMPju100+FuN6I9P2+Sn+Ehwo+w8ZZpyk34W5zlAQdG gsft65oyLZahCq7CufO0TPPTRULCbC9nnSPGd4TXGNDzcpBjBo8nwLzSR bwd6hIsHytg6TLKbDBQ9O7vgcKYO3V/YATqzaO5L5LMgaEuuy+rURbXQE iU2ikKfpP0aeTYRaxJn8ar4o5v/IZJHeIFsXQTbRvolR4SkRVe7vgWokD gqNiNm7/9w2JuvsnrT+n7xYefdrtC0sfn5dzNXuuE8dZhDW0+aPMNyOX6 g==; X-IronPort-AV: E=McAfee;i="6500,9779,10557"; a="316388245" X-IronPort-AV: E=Sophos;i="5.96,236,1665471600"; d="scan'208";a="316388245" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Dec 2022 03:40:33 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10557"; a="647864747" X-IronPort-AV: E=Sophos;i="5.96,236,1665471600"; d="scan'208";a="647864747" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Dec 2022 03:40:31 -0800 From: "Huang, Ying" To: Kairui Song Cc: linux-mm@kvack.org, Kairui Song , linux-kernel@vger.kernel.org, Andrew Morton , Miaohe Lin , David Hildenbrand , Hugh Dickins Subject: Re: [PATCH 4/5] swap: remove the swap lock in swap_cache_get_folio References: <20221208180209.50845-1-ryncsn@gmail.com> <20221208180209.50845-5-ryncsn@gmail.com> Date: Sun, 11 Dec 2022 19:39:34 +0800 In-Reply-To: <20221208180209.50845-5-ryncsn@gmail.com> (Kairui Song's message of "Fri, 9 Dec 2022 02:02:08 +0800") Message-ID: <87iliiqidl.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 2796F120006 X-Stat-Signature: 85pd4b34wggphykqidy3z85rbjifxiy8 X-HE-Tag: 1670758835-952437 X-HE-Meta: U2FsdGVkX18Qfe7PI8Zjpvn1V09e2SnrOtIsy9G/I2SwHdc+13SeC7+8m50K986QF7aDbnC016znlkSGyI/B9saJY/2jeZrWg60XZCEnBdDGBjD9syohuRxfs+gARK14sTcFra/p+vO9MeimPybFej5ezumvyfYnxMc0eOuP20EOLarqTAqeDuuBZre07d8OKV9d2iERq3PFEr+eusj5Fl1/GXUw7RjyKs3APp0ITXtUL4ge2l+3XqFGGtfJ3YTAESbBieW9pt0HmT3+XaZCXXVpHYKy5DUSbMhYBL9TypobJcdSmj1cMeMPL61TQ2GEUEIaEk0qvIe7OjT6xRaf5uGGJwi08XJeq1hXI4iqZ5ZvoKJkSCiIalJKVPKdc2sMt1yplnDD4bPSMZpqE6aDJNY9+UTQ0pxrUdW8yUovRxYqlumKn0IQkFe2zfh18TlSC1hJvJXEFu6MK8zofNOUwftdmxSW0jWD3Cgvi2IAw0UoEwsjWhE8F5/WWa4H9r0o3tUAUY/j1VcpJcG72DpytFSisQDe1h4t1E6kTrKvu6BH1Yt/n+65cXoJDJBsijzfWcNKNd6D4QpYnL61P6+N+uc7CpHM3aLiV68o130XaqMTpuprWY65h596ckuRhlmJ+7d13V/G0UpdHaicslJtyamrQojebs0X5MNEs5w66gACUi0ePq8JnA4qbjmYa+UzxMf0KnvG8eyxY9iE2jCGY/dsNYZ2ZP35KuFezGlWY9juSNc9j12Fv+boE7M6gvMGDASq9PG4fGrPEvq1lgie8UmTWf7PlWvpQfhKhkTjsl04QWTzgalcyiJukOEbUP59bYuggPp/zA/X0IIOcZ7lhOzLBMnedeMZ2iTBndaYhCfxMpoWfu+oIytWCwdjHj8hvlvKVy4BWxI9RnfmJaUNe26lQFXb+MBI 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: 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_folio I don't think you remove `swap lock` in swap_cache_get_folio(). Just avoid to inc/dec the reference count. And I think it's better to add '()' after swap_cache_get_folio to make it clear it's a function. > 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 = inode->i_mapping; > struct shmem_inode_info *info = SHMEM_I(inode); > struct mm_struct *charge_mm = vma ? vma->vm_mm : NULL; > + struct swap_info_struct *si; > struct folio *folio = NULL; > swp_entry_t swap; > int error; > @@ -1737,7 +1738,12 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index, > return -EIO; > > /* Look it up and read it in.. */ > - folio = swap_cache_get_folio(swap, NULL, 0); > + si = get_swap_device(swap); > + if (si) { > + folio = 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. 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 valid. > */ > 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 = get_swap_device(entry); > - if (!si) > - return NULL; > folio = filemap_get_folio(swap_address_space(entry), swp_offset(entry)); > - put_swap_device(si); > - > if (folio) { > bool vma_ra = swap_use_vma_readahead(); > bool readahead;