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 X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1CA4EC433ED for ; Sun, 25 Apr 2021 03:33:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7254961154 for ; Sun, 25 Apr 2021 03:33:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7254961154 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 040FA6B0036; Sat, 24 Apr 2021 23:33:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 017B86B006C; Sat, 24 Apr 2021 23:33:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E214C6B006E; Sat, 24 Apr 2021 23:33:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0190.hostedemail.com [216.40.44.190]) by kanga.kvack.org (Postfix) with ESMTP id C6F206B0036 for ; Sat, 24 Apr 2021 23:33:51 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 731101801874A for ; Sun, 25 Apr 2021 03:33:51 +0000 (UTC) X-FDA: 78069470262.16.0FB0E9F Received: from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190]) by imf24.hostedemail.com (Postfix) with ESMTP id 7FE9EA000398 for ; Sun, 25 Apr 2021 03:33:40 +0000 (UTC) Received: from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.58]) by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4FSYTh4WLVz17Rjs; Sun, 25 Apr 2021 11:31:20 +0800 (CST) Received: from [10.174.176.174] (10.174.176.174) by DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id 14.3.498.0; Sun, 25 Apr 2021 11:33:43 +0800 Subject: Re: [PATCH v4 4/4] mm/shmem: fix shmem_swapin() race with swapoff To: "Huang, Ying" CC: , , , , , , , , , , , , , , References: <20210425023806.3537283-1-linmiaohe@huawei.com> <20210425023806.3537283-5-linmiaohe@huawei.com> <87bla3xdt0.fsf@yhuang6-desk1.ccr.corp.intel.com> From: Miaohe Lin Message-ID: <0213893e-2b05-8d2e-9a79-e8a71db23644@huawei.com> Date: Sun, 25 Apr 2021 11:33:42 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <87bla3xdt0.fsf@yhuang6-desk1.ccr.corp.intel.com> Content-Type: text/plain; charset="windows-1252" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.176.174] X-CFilter-Loop: Reflected X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 7FE9EA000398 X-Stat-Signature: s37hgsg5bhohjco8mwxyqeziu478usb6 Received-SPF: none (huawei.com>: No applicable sender policy available) receiver=imf24; identity=mailfrom; envelope-from=""; helo=szxga04-in.huawei.com; client-ip=45.249.212.190 X-HE-DKIM-Result: none/none X-HE-Tag: 1619321620-424694 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 2021/4/25 11:07, Huang, Ying wrote: > Miaohe Lin writes: > >> When I was investigating the swap code, I found the below possible race >> window: >> >> CPU 1 CPU 2 >> ----- ----- >> shmem_swapin >> swap_cluster_readahead >> if (likely(si->flags & (SWP_BLKDEV | SWP_FS_OPS))) { >> swapoff >> .. >> si->swap_file = NULL; >> .. >> struct inode *inode = si->swap_file->f_mapping->host;[oops!] >> >> Close this race window by using get/put_swap_device() to guard against >> concurrent swapoff. >> >> Fixes: 8fd2e0b505d1 ("mm: swap: check if swap backing device is congested or not") >> Signed-off-by: Miaohe Lin >> --- >> mm/shmem.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/mm/shmem.c b/mm/shmem.c >> index 26c76b13ad23..be388d0cf8b5 100644 >> --- a/mm/shmem.c >> +++ b/mm/shmem.c >> @@ -1696,6 +1696,7 @@ static int shmem_swapin_page(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 : current->mm; >> + struct swap_info_struct *si; >> struct page *page; >> swp_entry_t swap; >> int error; >> @@ -1704,6 +1705,12 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, >> swap = radix_to_swp_entry(*pagep); >> *pagep = NULL; >> >> + /* Prevent swapoff from happening to us. */ >> + si = get_swap_device(swap); >> + if (unlikely(!si)) { >> + error = EINVAL; >> + goto failed; >> + } >> /* Look it up and read it in.. */ >> page = lookup_swap_cache(swap, NULL, 0); >> if (!page) { >> @@ -1720,6 +1727,7 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, >> goto failed; >> } >> } >> + put_swap_device(si); > > I think it's better to put_swap_device() just before returning from the > function. It's not a big issue to slow down swapoff() a little. And > this will make the logic easier to be understood. > shmem_swapin_page() already has a methed, i.e. locked page, to prevent races. I was intended to not mix with that. But your suggestion is good as this will make the logic easier to be understood. Just to make sure, is this what you mean? Many thanks! diff --git a/mm/shmem.c b/mm/shmem.c index 26c76b13ad23..737e5b3200c3 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1696,6 +1696,7 @@ static int shmem_swapin_page(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 : current->mm; + struct swap_info_struct *si; struct page *page; swp_entry_t swap; int error; @@ -1704,6 +1705,12 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, swap = radix_to_swp_entry(*pagep); *pagep = NULL; + /* Prevent swapoff from happening to us. */ + si = get_swap_device(swap); + if (unlikely(!si)) { + error = EINVAL; + goto failed; + } /* Look it up and read it in.. */ page = lookup_swap_cache(swap, NULL, 0); if (!page) { @@ -1765,6 +1772,8 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, swap_free(swap); *pagep = page; + if (si) + put_swap_device(si); return 0; failed: if (!shmem_confirm_swap(mapping, index, swap)) @@ -1775,6 +1784,9 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, put_page(page); } + if (si) + put_swap_device(si); + return error; } > Best Regards, > Huang, Ying > >> >> /* We have to do this with page locked to prevent races */ >> lock_page(page); >> @@ -1775,6 +1783,9 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, >> put_page(page); >> } >> >> + if (si) >> + put_swap_device(si); >> + >> return error; >> } > . >