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 CBC63C7EE23 for ; Mon, 22 May 2023 01:28:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6565E900002; Sun, 21 May 2023 21:28:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6062B6B0075; Sun, 21 May 2023 21:28:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4CE8C900002; Sun, 21 May 2023 21:28:15 -0400 (EDT) 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 401206B0074 for ; Sun, 21 May 2023 21:28:15 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 1412616015C for ; Mon, 22 May 2023 01:28:15 +0000 (UTC) X-FDA: 80816155350.27.8942842 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by imf08.hostedemail.com (Postfix) with ESMTP id A9459160011 for ; Mon, 22 May 2023 01:28:10 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=HuVOLrqX; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf08.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.151 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684718892; a=rsa-sha256; cv=none; b=QE8z+i8VawGf2o0NG4jbaXa7RAirbQpsX+95Iyrk2PiEDpw69+8JeJsVglRWJsdC6+UNcq LeE0j43jdnK1o+HiWTBTbp7BCp2yfr1alnzuwSxboAHukDIqDFMoR85P9wYvkdAkSvM1/E EEC/UCkzYA+GtuHybjtRkzaQtCmProw= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=HuVOLrqX; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf08.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.151 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=1684718892; 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=OWEBjFpl8tGxRb4A58LM0O811nOIsHcpiFyPt0bFl5E=; b=3fFpQqNjVXH+b51acSeGhJkASTMyeTkEKPIeDmVO+IoM2fPOmr/JQlXGB6Nx0ULXplh+6K RLzg1JtfU9YgS/uuWKT3lw2KXLfHboTvcaIA/YF47oIZqPSW6YwtPLj6D8IM6OA/fLworc iiMlv/JMMIIivI2r00jHBYBxJFLxumk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684718890; x=1716254890; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=QyQJCN5elPlHL3m/otcd0ae3nywiqlEjaHw9qMhPzo0=; b=HuVOLrqXDdxmkgF656dvkzJpN/60Y+9xxzeYZDe2j4k/7Y0LFZEZjxnc 49fl9evetlWZlxsNvBwCPUlPwSuSqcdNsWV+N68dSOAUcFuH0yaMIMhfH 0rqtwdXiB0oIo9pXMN3bfx8vl6k5Syzh4mLuYOk3Jy8OswVXgIkG69wpB K5PiAGkcRtsmm+uj96p0qfYLvIaBR1FhbfsHLPP8hb1DHOV2WXOuZxkZp E91mpwVXg3AjQyIAS3pIjD8VPAvuGSG3XiXoCrDCKzqy4n0/1CiB98Kn9 +h3VrmGAIxMo9NSvPoN5fK6Dhq4un3stMx3XLNt17JztznIIXlI/bL/jv A==; X-IronPort-AV: E=McAfee;i="6600,9927,10717"; a="333165989" X-IronPort-AV: E=Sophos;i="6.00,183,1681196400"; d="scan'208";a="333165989" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 May 2023 18:28:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10717"; a="768325617" X-IronPort-AV: E=Sophos;i="6.00,183,1681196400"; d="scan'208";a="768325617" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 May 2023 18:28:06 -0700 From: "Huang, Ying" To: Chris Li Cc: David Hildenbrand , 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] swap: cleanup get/put_swap_device usage References: <20230516052957.175432-1-ying.huang@intel.com> <87fs7v7qmh.fsf@yhuang6-desk2.ccr.corp.intel.com> Date: Mon, 22 May 2023 09:26:46 +0800 In-Reply-To: (Chris Li's message of "Sat, 20 May 2023 09:40:30 -0700") Message-ID: <87ilcl2m21.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: rspam06 X-Rspamd-Queue-Id: A9459160011 X-Stat-Signature: g1zk3fou4d5r9wostpj5wmcgkwrc453b X-HE-Tag: 1684718890-504550 X-HE-Meta: U2FsdGVkX18fYIoXopstmRbXhPkG/MAVSFuw0YjAk+LVSJfZE9OYH5KpobiorMi23/wPU06iNTOHGRkyiG68vxFYS7z/W+HMiocx26J3DyowB6LvHQwVDSpiuUiZURpjooh4tf4kFGUa4P/ag3/UstkkpG6MiqQZDewEjAVwrgwOF1ksP8K0XYWXTZk7mTjpHYBSoHk6oRZ+e+sZMMwzrJWBalqgCAvMYoG8MOrpxGxDzIxre7xxg+S8ZIE32VLYDXw1XFz5xyBthcdf17cnLqxeELy0QOb7zVCC2TM3e0/4izSxJ97vB7OHrgVMEZpA8cSAemVC6yKuL8qHBqt6DJs0g63fa9+yhPK5JX/HHIDtsGJT4pelk+Vzd8AOxJx41QDUEwi0jth+f/jDfN3lOnqIG/H2lQw5CfZ2HL4S0m0KdvcNRPDa8oaaD1DJBc3LqT/G2AVdiK+9nm19ZJhqf5DTCrGnaMskq9Xomv89k8BVPKo8MyMMZilKn/XaZtFiaXDuJVD3lB1KWqLLGU8+OEtwiF493mgUcr2Gml6J2bLPAjrrMpH+6wz59p87MNDKXPSuLJEAywiAp8u1V52TA93Q4zl0suTv29+7ykKjyEhFNtz03whTYnG33yYwVySayiENmSCilvdpU7dY8uVjte4XDQznzcQ1wIcCAEL4tnNCIogTq177XCzdlBpACZbfKHoSTBM5Jfkn46ajjsEzBgmzUxeGg8hbtG7ibt93x6BJg/ftwQ2XOdqcibsH5T0/7I32a1NWXUdsCX+MVhVYcT8NgDYyhIC9ull7cvoLDzlkf2aizPi+1GaOT/KYWLWuXaSNsBFU07xGQt/8sBW7pw9+R6oUEpF1zf1z7k+6FikYF/diG+AEImmGYB7AGUTwRENsRXmncvnys/ZHJuwB5VXtEOg1JLFmGVLoyVyktXk7dpb5j7pKT43o1uEs4WaJA5k1r2ctyl4Eu34Xv1K UOC/T3SL 9CqDEqiwmILRl9dLX91D7Zs3BRjBWziwiGcTIfHlnxuUevp4w/vlSOxy1mV1OFM9e/5MhTDBxvr3o2OcVavd7HSzi7bBet+L4al8W0SF+zn/rLQ3hT+1N6Yvz6kZHKGLEGYDtQTBeB7GcuiUKFzZC8Su3cw0CUT/m+DV4CcFX/0vJq6CqwesE2h73MTBygnOYVqItOB9F/JzhEXK7oNUDscrvZFCxtHepSK6jWZuQsaJXVjJIBC3+p0ozfXEZzKbCJB6iif4oBZYOtCVBVXfrmu6HVg== 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: Chris Li writes: > On Wed, May 17, 2023 at 08:23:18AM +0800, Huang, Ying wrote: >> David Hildenbrand writes: >> >> > On 16.05.23 07:29, Huang Ying wrote: >> >> The general rule to use a swap entry is as follows. >> >> When we get a swap entry, if there isn't some other way to prevent >> >> swapoff, such as page lock for swap cache, page table lock, etc., the >> >> swap entry may become invalid because of swapoff. Then, we need to >> >> enclose all swap related functions with get_swap_device() and >> >> put_swap_device(), unless the swap functions call >> >> get/put_swap_device() by themselves. >> >> Add the rule as comments of get_swap_device(), and cleanup some >> >> functions which call get/put_swap_device(). >> >> 1. Enlarge the get/put_swap_device() protection range in >> >> __read_swap_cache_async(). 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 >> >> function called by __read_swap_cache_async(). >> >> 2. Remove get/put_swap_device() in __swap_count(). Which is call in >> >> do_swap_page() only, which encloses the call with get/put_swap_device() >> >> already. >> >> 3. Remove get/put_swap_device() in __swp_swapcount(). Which is call >> >> in __read_swap_cache_async() only, which encloses the call with >> >> get/put_swap_device() already. >> >> 4. Remove get/put_swap_device() in __swap_duplicate(). Which is >> >> called >> >> by >> >> - swap_shmem_alloc(): the swap cache is locked. >> >> - copy_nonpresent_pte() -> swap_duplicate() and try_to_unmap_one() >> >> -> >> >> swap_duplicate(): the page table lock is held. >> >> - __read_swap_cache_async() -> swapcache_prepare(): enclosed with >> >> get/put_swap_device() already. >> >> Other get/put_swap_device() usages are checked too. >> > >> > I suggest splitting this patch up into logical pieces as outlined here >> > by you already. > > Agree with David here. > >> >> OK. Will do that in the next version. > > Your patch make sense to me. > > Looking forward to your next version. > > BTW, no relat to your patch, but just when I look > at your patch I notice is that we have too many swap > count functions. > The naming scheme is very confusing. > > 1) swap_count(), just mask out SWAP_HAS_CACHE > > 2) __swap_count() the name with underscore suggest it > is more internal. But __swap_count() calls swap_count(). > It is basically swap_count() with device lookup. > > 3) swap_swapcount() > similar to __swap_count() but with cluster level > locking if possible. otherwise fall back to device level locking. > > 4) __swp_swapcount() > swap_swapcount () with device lookup. not consider continuing. > Again this function is more external while swap_swapcount() > is more internal. > > 5) swp_swapcount() similar to __swp_swapcount() > exact count consider continue > > We should have a more consistent naming regarding swap count. > Device level, then cluster level, then entry level. Yes. The original naming is confusing. > Also I consider the continuing is internal to the current > swap index implementation. If we have alternative swap file > implementation, we might not have count continuing at all. There's some difficulties to hide continuation completely. For example, we want to call add_swap_count_continuation() in non-atomic context in copy_pte_range(), while the fast path calls swap_duplicate() in atomic context (via copy_nonpresent_pte()). Best Regards, Huang, Ying