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 212FFC77B75 for ; Tue, 23 May 2023 00:44:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B4EDB6B0072; Mon, 22 May 2023 20:44:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AFF1B900003; Mon, 22 May 2023 20:44:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 979E6900002; Mon, 22 May 2023 20:44:26 -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 8922C6B0072 for ; Mon, 22 May 2023 20:44:26 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id BD2F3C0535 for ; Tue, 23 May 2023 00:44:25 +0000 (UTC) X-FDA: 80819673690.04.6334A57 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by imf12.hostedemail.com (Postfix) with ESMTP id 0147940008 for ; Tue, 23 May 2023 00:44:22 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=G9LHftZO; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf12.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.136 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=1684802663; 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=pNSLigJMCW8WAGcfrk2OvjDJbJ6gLhRc1TMQwi9xQjM=; b=XcNZzuKCdrXIaCm/i+7RnK7kSa+WcADV31vBn6GCdjVj+ZSudGBYZu/K8k6oDllcisK6lv IHgj62bs80ysfEeMTENxJ9eqlksrLw1vZbJyMpCghvhsu6+rjCgYIhninU0BsA+S5lYb5J BLjV9idwYVc5Xf3yMiS9V/I6ieyymKQ= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=G9LHftZO; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf12.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684802663; a=rsa-sha256; cv=none; b=EgquliLLHJEnxth/lIv5cXqBER/4smZ85K002EY/6oYx4FnQxqKy5UCwPz5IIPADmvkKPz rst26ejkOV202cGrZOEA8d8raAoduRScwljx1FxV12DMXY/XTsxVwdrcqrpHDiINhOKNmD 0tYKRgxNFT8MJEneta/+G9AAeCSr8Oc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684802663; x=1716338663; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=Zw1KmdjsbS5Kd4VsbrQGPX4PE9kzdomn3cNjKEJ1Y9g=; b=G9LHftZOGLm7FRY5JGPGlK1c+c2Gy7VzL0Md/okaMoOCqHXcl3IBJqvc DrCZsCAEM3Nc2jo8FaMI0duDEhhm6OcynMeJq7VEaKO2jvck3Teh9OQDJ R/GIktFJjtFMHjoozYOOo9rRyGJU39Z/+FltoMboRaAiud9w8dStQbJYi XKHPytiAAV0P4dE42y+JYMvX0XOtnEtXKyL+zv4VrlzhyBwhsoZF0Kq7s TTDAl6CJE+/hx2wUT5UvoAiSrxHV3ruc3caaGOJiwJJ/HLGqmpeZH2tP1 YJugC5HgdFnQeAdPRX4WeRf15GQ/48krFZ5PUMA/vDioCudhkhYsacGee Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10718"; a="332697757" X-IronPort-AV: E=Sophos;i="6.00,184,1681196400"; d="scan'208";a="332697757" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 May 2023 17:44:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10718"; a="703748487" X-IronPort-AV: E=Sophos;i="6.00,184,1681196400"; d="scan'208";a="703748487" 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; 22 May 2023 17:44:18 -0700 From: "Huang, Ying" To: David Hildenbrand Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hugh Dickins , Johannes Weiner , Matthew Wilcox , Michal Hocko , Minchan Kim , Tim Chen , Yang Shi , Yu Zhao Subject: Re: [PATCH -V2 2/5] swap, __read_swap_cache_async(): enlarge get/put_swap_device protection range References: <20230522070905.16773-1-ying.huang@intel.com> <20230522070905.16773-3-ying.huang@intel.com> <653a4881-d17b-6d8e-8066-300c0497a702@redhat.com> Date: Tue, 23 May 2023 08:43:15 +0800 In-Reply-To: <653a4881-d17b-6d8e-8066-300c0497a702@redhat.com> (David Hildenbrand's message of "Mon, 22 May 2023 14:01:32 +0200") Message-ID: <87wn0zyj18.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-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 0147940008 X-Stat-Signature: 3fqnx4fjbcdgcjtw5mm7ia7umsk3ckq7 X-Rspam-User: X-HE-Tag: 1684802662-127543 X-HE-Meta: U2FsdGVkX1/dLI575cCmXPP8yy8CzZ+scM4v1CQh7crk5yHFUXRHusIWDPu+xmfIMLFBtgqiKhBgYHvAUMI9KboqWM1ZoV9cmjtrZMeh8H/lzQkeHZ/GNH8GAvuhfdsdNJ0Ha3yALxGPiGvd6ugTZ1zFpDP5eDuMlBKRty5CQwMwzHmkPJ99eyhq8oxDMC8tHdmp6xbVn5lG/atuB46wVCBnyZlurfBZoQMsEvq2845WgZsOALnmGQOZms9a647XLndkoae64A6SVn+Gq7d1yy95P8qQ1Do2OdLbbVAqKX7Gar+5SUiPzksa3BUxMdg88LC9JUyUOCyQITxH6JXnhCmhtFrtkq+J8iivO/i/Vtcn5UgABBNKBCMnnXBwexVDZ+xuQ4jlxF8i628TaxS1eY3cES4QC6ItyHMcdUJ0Da40gmzLvqnxnqBbpSPVt+jTVFNGIWpTnC0G267nDHlTRkj5WRHkvpaYDbjCbrmXvhtllpNp0GTjHXuB4FYGKZnkkXqiimWx1DHIa/C8TnWYGdA0wuZf6ApmfLoQ1iaFhaPoaDRZ/Tq7jyragIaysb7rioQ+vfY4G/r2R45DW4waX2YFXoPtUUNlANiA/CkOYQlww2oLg8ndGi76s/1CjglEwKQnbfB6iaVWGM/WNroAIehCveaDUuXb892S+YzlajmhGLV0UrLIUnGY5HkHq68w6XmPh7kIiCinj1Xhs1Dtd2PnARa74xBWvWWo1neg0uQBIpmSjtQ0fA+1CbLJguyEigfnSQG1MCl3nVXLT6Znq+1WEbArzsERYDEyjdjduQPUMVHnts8f/4E1s6ySMv4u4/mmiwdcgEPF0Z3MVRPfzh/wTsTH1Af9eUSKfqh0qkcyvWOkJn/8iA1kbZ8c/WrYGlHo9v5/2WaY/qi1SqDH9UM6OZosH99tDXZA30nBgXkPe+FU4JSLxKoD4iHP7Mimp76XffCDKE8ASuyIEei tJuZxAfj 6RTG5QYHH2tAkojnJennV6IyuhlrAOzPMVZRWu+4cDnFkzkSMcHv5gDwbxvjLMqEPqqA+nSmik3NiAs4eJup9vd9GvwYUgM2cV4jBxnjEnNFgYLlGm3kfkLLQYHUIsqJ4hru98dphNBU6ZkAe+uip3Lfn3Q/7upDaU5QpXAWaXQnQ8omte7/8l50Ul4EoHKmRw/g1DSgSde9AbQKWDmqV/4p4AuXOG5DP1Wq6RH+T75jtaXGhlwcXkYIUMIZSYX+n6TWPuJgKY98ruVii/m/Kfsc+WlFf1JulsYa46UR9wlLRT0DBawQSZMyiUXt+nqJz1OTsgzzxlnYtAmQ1UwQ+eVs+JABQk5HmrZp4VUDdysQZyA9jhgqJtViDgOS8sDIosbwhxfgOElr1duq01KD7pXKRSE51mIlee8XY4DT77TuOwfrZodyLzb1XgaTi+MzOERdm0hgnCwJxbRzrJrwerRO0W6ZwZns3vKNvL5mK98gNPDw09gQtFw708w9q7zfOQW7ViWHwhwO4VOQ= 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: David Hildenbrand writes: > On 22.05.23 09:09, Huang Ying wrote: >> This makes the function a little easier to be understood because we >> don't need to consider swapoff. And this makes it possible to remove >> get/put_swap_device() calling in some functions called by >> __read_swap_cache_async(). >> Signed-off-by: "Huang, Ying" >> Cc: David Hildenbrand >> Cc: Hugh Dickins >> Cc: Johannes Weiner >> Cc: Matthew Wilcox >> Cc: Michal Hocko >> Cc: Minchan Kim >> Cc: Tim Chen >> Cc: Yang Shi >> Cc: Yu Zhao >> --- >> mm/swap_state.c | 33 ++++++++++++++++++++++----------- >> 1 file changed, 22 insertions(+), 11 deletions(-) >> diff --git a/mm/swap_state.c b/mm/swap_state.c >> index b76a65ac28b3..a1028fe7214e 100644 >> --- a/mm/swap_state.c >> +++ b/mm/swap_state.c >> @@ -417,9 +417,13 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> { >> struct swap_info_struct *si; >> struct folio *folio; >> + struct page *page; >> void *shadow = NULL; >> *new_page_allocated = false; >> + si = get_swap_device(entry); >> + if (!si) >> + return NULL; >> for (;;) { >> int err; >> @@ -428,14 +432,12 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> * called after swap_cache_get_folio() failed, re-calling >> * that would confuse statistics. >> */ >> - 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 (!IS_ERR(folio)) >> - return folio_file_page(folio, swp_offset(entry)); >> + if (!IS_ERR(folio)) { >> + page = folio_file_page(folio, swp_offset(entry)); >> + goto got_page; >> + } >> /* >> * Just skip read ahead for unused swap slot. >> @@ -445,8 +447,8 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> * as SWAP_HAS_CACHE. That's done in later part of code or >> * else swap_off will be aborted if we return NULL. >> */ >> - if (!__swp_swapcount(entry) && swap_slot_cache_enabled) >> - return NULL; >> + if (!swap_swapcount(si, entry) && swap_slot_cache_enabled) >> + goto fail; >> /* >> * Get a new page to read into from swap. Allocate it now, >> @@ -455,7 +457,7 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> */ >> folio = vma_alloc_folio(gfp_mask, 0, vma, addr, false); >> if (!folio) >> - return NULL; >> + goto fail; >> /* >> * Swap entry may have been freed since our caller observed it. >> @@ -466,7 +468,7 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> folio_put(folio); >> if (err != -EEXIST) >> - return NULL; >> + goto fail; >> /* >> * We might race against __delete_from_swap_cache(), and >> @@ -500,12 +502,17 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, >> /* Caller will initiate read into locked folio */ >> folio_add_lru(folio); >> *new_page_allocated = true; >> - return &folio->page; >> + page = &folio->page; >> +got_page: >> + put_swap_device(si); >> + return page; >> fail_unlock: >> put_swap_folio(folio, entry); >> folio_unlock(folio); >> folio_put(folio); >> +fail: > > Maybe better "fail_put_swap". > > We now hold the swap device ref longer than we used to, prevent > swapoff over the whole operation. I guess there is no way we can > deadlock this way? I think that we are safe. In swapoff() syscall, we call percpu_ref_kill() after all pages are swapped in (via try_to_unuse()). > In general, looks good to me. Thanks! Best Regards, Huang, Ying