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 00EA7C5478A for ; Wed, 21 Feb 2024 20:36:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 854046B008C; Wed, 21 Feb 2024 15:36:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 804466B0092; Wed, 21 Feb 2024 15:36:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6CC4A6B0093; Wed, 21 Feb 2024 15:36:14 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 5AFDB6B008C for ; Wed, 21 Feb 2024 15:36:14 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 308FF40B33 for ; Wed, 21 Feb 2024 20:36:14 +0000 (UTC) X-FDA: 81816968268.26.9770CD5 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf11.hostedemail.com (Postfix) with ESMTP id A89DD4001B for ; Wed, 21 Feb 2024 20:36:11 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=NhXTm9Ss; dmarc=none; spf=pass (imf11.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708547771; 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=RmvCzu/yUeqVtryQzxOkj9l9Tdhl3SMCOD5K5IFbqUI=; b=IqcxWNGSyL7pmB7Y3NquBqEz0pGdl/kKIiEn8gO2tUOh9J9qiLzpDAgBTjS6JG1pebwvi/ SkftDV0BU/ttKXie3BDqJilIwQTnkzQzXjHml16XjZM6gyXgvP2Ml9OcqnbFAYMBs4U1Yw acbmkdgyB0VZhiILaE76jyezWqMrIoc= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=NhXTm9Ss; dmarc=none; spf=pass (imf11.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708547771; a=rsa-sha256; cv=none; b=QMGDoBpBXDlY1/ymy5IpPye3no2P9dM9xm8RBdKduUnVvtiMETOHj/xRSfBJcJSSWiXNQe wSPZN8uHEffoX44mL3QbbhDnkFXNep8EZp5CHw61jg15W/2hw5XCX53v7RK32dX4kOq9UQ 1p6WWv3WtjCgP/3fsSR6zEaW/1PZyZU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 941DA61500; Wed, 21 Feb 2024 20:36:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0DFE0C433F1; Wed, 21 Feb 2024 20:36:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1708547770; bh=hWY1Y7gwaHsCt+60O0plbbDiTxgt1/isDaR9hySz3Bo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=NhXTm9Ss+RtkoPhR9wtAeLEzXXMWA+4u6rw8ILNyykZ9foBpB4ny8n/PFimlKdEY6 LXlO6hlrVfgSbm0HQ6HtxVv8I17FVhBaYhkC+6xF8aZb7w7ZUoNalfj4gSNTZz1NCg tKwNt2lg7LFKJR3fvlZcAtPNbNG2IXowaer4P7WY= Date: Wed, 21 Feb 2024 12:36:09 -0800 From: Andrew Morton To: Barry Song <21cnbao@gmail.com> Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Barry Song Subject: Re: [PATCH] mm/swapfile:__swap_duplicate: drop redundant WRITE_ONCE on swap_map for err cases Message-Id: <20240221123609.3cd20c3dc2d6adeaf5d3ffc8@linux-foundation.org> In-Reply-To: <20240221091028.123122-1-21cnbao@gmail.com> References: <20240221091028.123122-1-21cnbao@gmail.com> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: A89DD4001B X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: atykarsfs6n8bxhqyyz9mfxbhec4qui7 X-HE-Tag: 1708547771-942591 X-HE-Meta: U2FsdGVkX1+j7x6C88UmlFDqFENct5BizURGM+ncg42cFw8y0yJbVEsq/x7y69wTkgo+OoszKmypftzmJLG39BByHA05WjqPlJ4inkjoXwJjbdQQ6mJM89XeXGCUbbQkJt/URK7VhveJNAhTcAERal9OzzL96gIly+vdhdmudBceyJGXbIyMNPpnENZi1zDBHdosVpm6q8DZsDBp7OioevcRfU87URSrv1n6HUpVR2zJLSZl1YF166GGhZolkLqINCLZMv0h3XZCHBVfOXAUbVIWApUEZS64edm8+bM8/a3B/5FhfI01to9hPp0vN0eZVGjTmEXd0PiggZPA+DFCJLrQQAY0O/6e3GfL0G7G5s16X7Z/JifbQNtxKvdOE8i+uLgS8n66d6zRJ1RNhQKdnzMrZueu+ddnXGscVTmOUic+cWTIJ65oZhlAdAgLRVaud9uxi/rZykafdqmvpZIvIXal/Qsy0a14V6Y4XoXpz4C4+mfyjkPAgUvbKYknURt8DsGEAGouYaDXmgpesWn8x/oFYUyYAJltiicXxEhw8ZIiuHdMpzUC2kSTDmwcUyXwtzMKtjzxVdkWqV996DgSNEZh8+DTKpHx/kuJY09qkfh4DONL5JJ8OCSLx5C7OgQtXawfLZBDYc4uJONQagFRn/Bhp2xv4Q7OZ5Y0lImWdM/C2pzE6YU4156rQUwpOd/KZQdVX8d59uJ/klc8QwX+qbQzAHcmybI1HK4yV1uWvKTM026kTH+bfU3QIv6Eqhial5kUsC8P9Gbjn9u+51z+wvuL4Ya4k4tcxDXH50/1O1qANWq5oV3w6ucP+9up0mB5gNZ4vmOGXIoM4nsfOiG8gXulJ1Syd8YMGelvJctTo5JP7QYs4PdgE9oASmzTXmhK+wBL7GbgdAAIbhUWO2wZz9VQtLqVBa9iFQzBno7Gwg8TELlBF9/KmYEV4sw0J5tLeBYoM1OOMchnbx4azq3 52nQJRcM 0banz 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: List-Subscribe: List-Unsubscribe: On Wed, 21 Feb 2024 22:10:28 +1300 Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song > > The code is quite hard to read, we are still writing swap_map after > errors happen. Though the written value is as before, > > has_cache = count & SWAP_HAS_CACHE; > count &= ~SWAP_HAS_CACHE; > [snipped] > WRITE_ONCE(p->swap_map[offset], count | has_cache); > > It would be better to entirely drop the WRITE_ONCE for both > performance and readability. > > ... > > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -3320,6 +3320,9 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage) > } else > err = -ENOENT; /* unused swap entry */ > > + if (err) > + goto unlock_out; > + > WRITE_ONCE(p->swap_map[offset], count | has_cache); > > unlock_out: I agree, but why add the goto? --- a/mm/swapfile.c~mm-swapfile-__swap_duplicate-drop-redundant-write_once-on-swap_map-for-err-cases-fix +++ a/mm/swapfile.c @@ -3335,10 +3335,8 @@ static int __swap_duplicate(swp_entry_t } else err = -ENOENT; /* unused swap entry */ - if (err) - goto unlock_out; - - WRITE_ONCE(p->swap_map[offset], count | has_cache); + if (!err) + WRITE_ONCE(p->swap_map[offset], count | has_cache); unlock_out: unlock_cluster_or_swap_info(p, ci); _