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 4A760C61D92 for ; Wed, 22 Nov 2023 05:15:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B9A1D6B0556; Wed, 22 Nov 2023 00:15:40 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B227F6B0557; Wed, 22 Nov 2023 00:15:40 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9C1F26B0558; Wed, 22 Nov 2023 00:15:40 -0500 (EST) 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 8C0EA6B0556 for ; Wed, 22 Nov 2023 00:15:40 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id ED7671A03FA for ; Wed, 22 Nov 2023 05:15:38 +0000 (UTC) X-FDA: 81484427556.28.6DC6201 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf29.hostedemail.com (Postfix) with ESMTP id D9FB0120006 for ; Wed, 22 Nov 2023 05:15:36 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=N8GUQTDJ; spf=pass (imf29.hostedemail.com: domain of chrisl@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=chrisl@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1700630137; 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=QFI87lWQwCU40XUe+S4f1ebQgdGXRNK6x+bzjOEYjmk=; b=KjjKzAfolP9Jyo/rQEsAzabmeboIl6H3VNRiT+tVM2eK724C3/gwWU5jj8aZOY5gfaROcQ Tgmdob8kqTm0g+TxLxW787Ea0fVO+nD0lYAcbrOuBgsyU1BKPEm03qatEShcwxupsaY7iy bSE5IDLXADu6D+sjCN9TvyP7fJn0pX8= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1700630137; a=rsa-sha256; cv=none; b=Lda/eQsbRlN2hiMOhnspx8B1aZERbPJ4/WR0gVs0QUQpBMWjTozJTj4sYdkNIVaSMOexey pmXw5OabYY9NXLhITkcrw+P145J2nzOyGrkcuTx8rF3EljerdamTbW7yb/Kka4NlWIhGB/ ow/fLabJE4f1TQWQpfazBELbbfu1guc= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=N8GUQTDJ; spf=pass (imf29.hostedemail.com: domain of chrisl@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=chrisl@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id 275E6B82534 for ; Wed, 22 Nov 2023 05:15:35 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AFB1DC43397 for ; Wed, 22 Nov 2023 05:15:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1700630133; bh=8yQOtqhJDbJqJorbHOFHuOJO2GdlJMbA2UvGLCtMCfw=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=N8GUQTDJrBHnViR8kmwBc7GLdhYNNTLs6n0VJZrrLD2stFjGfiq16elnPlxnbQ1Is utHT21XjjdPT02HRZ1MRhJDnX+JE2TMOqCe54xcH86gjqfVGmATK97Dlne489i05ur QlImiXkNMESgHn54FB7OIYt7TH2trSdPbj1nr7fOS/PxInjzZyZqV6yeUOP6SYS6Y+ Ypb1SlQtLJBg4TSmRhaz0LC7gkKtPiuyDSfCRu3NJwVCVu8QonhIP7WQ/w7ZfaZ8eo H+woImxXZcMU9SaLo3PzrR+10LC6P1JrPvGW0kojsPr9X38lh8bDmzwFHD8jfZNJzo oHU6dF5uxiThQ== Received: by mail-pg1-f175.google.com with SMTP id 41be03b00d2f7-5bddf66ed63so358250a12.1 for ; Tue, 21 Nov 2023 21:15:33 -0800 (PST) X-Gm-Message-State: AOJu0YzB/uNrjfqaUroQ5/6Ad+cvhJYzD+TAxX7j9cAxZGzV77L2a9rL UhXZ7wfWgvMJ1v/iXoG1w10tPJwT3fFMyMubcSTQrw== X-Google-Smtp-Source: AGHT+IEa0OQTgkVX+7OZdO/Lyluv4nYZ1dytNefxGRLhdNMg7CjH0/ntePhtiyQ5RlEPm7/jzJ8DB+qPQAkITVmO0dg= X-Received: by 2002:a17:90b:1bca:b0:27d:8a04:f964 with SMTP id oa10-20020a17090b1bca00b0027d8a04f964mr2097699pjb.24.1700630133034; Tue, 21 Nov 2023 21:15:33 -0800 (PST) MIME-Version: 1.0 References: <20231119194740.94101-1-ryncsn@gmail.com> <20231119194740.94101-22-ryncsn@gmail.com> In-Reply-To: <20231119194740.94101-22-ryncsn@gmail.com> From: Chris Li Date: Tue, 21 Nov 2023 21:15:22 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 21/24] swap: make swapin_readahead result checking argument mandatory To: Kairui Song Cc: linux-mm@kvack.org, Andrew Morton , "Huang, Ying" , David Hildenbrand , Hugh Dickins , Johannes Weiner , Matthew Wilcox , Michal Hocko , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: zwr43a7g1hm3wwu1o4bjgmhjymp6hiow X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: D9FB0120006 X-Rspam-User: X-HE-Tag: 1700630136-360220 X-HE-Meta: U2FsdGVkX1/caUsVuAdlYiSpl90WPXmx5CCG4INqiqTW2BxtWHaBmE056gunkEZAtmHPm0sO13PN4jnFzIr/q8Vg2Db/0cWkxTbbGmQYF29v2PBctGwv/E63H0akFlKe9K3crdlMkpXo6XMxAV6MnBI64wOcWmnfC45dlEUeVhTvP5p8mPx7wzprhwkmiMcMNbddYI+HtmskTSk3uPyCJP00zY0evlUHczD+AemPNc5R1UXRq90AOrN1w+DBwyZOOQ/UU3M20MufCqbwjYSaU3D4c1WRHuDTIs84UowVJfdeQYrdyH+Dy173V+cYr5h5P4oQ7ktljFbIbKLj1huAHrWD2+llTnQe8SLBT/eGcyZR6k50PT+vxStk1yJR/VAcu1TE+ODaFer1i/iokLtmnlAU7RLMvyQ6HhH8lh/9e01HMISEvYEmliUKf22lJj9Ipa32vWDPzEXCoK/jUpe3rLrHxQxq5gJh8906Wk+vigE6cz/+YUwP8rknLwT2l9vYbCFyqMzhhUFKqsmEQ5jstOtrxID8+xDWX9nLerNQMvxUzZI1Vk+azetkTUBRWXWVmew116uhsB032vtzrREh5vPqL+pboXJSh8Aduko1BVcsS4ToJQ0dD1LMMuSt7/DnqyVdVYZnM084RpbwNdnv5SFWXw9mHSGeYqaod9KmVbrv7h8oa1ldWFSA1TJjSI68uhAYDr3lFI1er2ONnTxKn3otQfBEGHg5tDWXqfX0nyPuBfDkfeiVpLSo5JIF9LQ360nvD7JrihB0dC87vrt2nhV3WJ67lplc8cLEp9GZSSP6X5A15C59ewUk/HEJyGtyK3y19dFbOJ4bSsWtHGWKvxgMtsolvGdqXEjYk5V/hKQhCZQZXt30dow8wPkMcoN/3WdhPD9FP7L64QlVd2T/WEI74Ps7ZHe6oZ0qBFQRWwDj36rlikrN4XWqd3v1LfJv9eCEwKf2za3FPFfAI2G s1fDTvaG ZEgFU9hsuV8jzjHRFX81lYMqJM4VXN5lGCXmLPrERLUKRlXjWh+mw4wag7q5q+dpJIncKhFz9ORpmqNF3dOXcg24LY1yNYQB03HkiHdSIV75TCz6HM7iG8DInZQYuonXQWtuC1KAMna/x+22ot/AT9WpOZKpGlh/gt3y5c9IwD7HZAcmJX42W7GxhjkoKGz/KBiDVCvbVMnSwgJpsJmFPrS6CQ+KJ6fjidKBNLUqNFCIqPcDIvd6DZwsmG3vl+AKnc9XHY+j4fE9bEbURx9+3ikaM9X4jV7Bls08N3yRj3AjymE1t9CGqarW784LBrp133WHt6tiJOn60Ndf0Kg0KSLoqVg== 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 Sun, Nov 19, 2023 at 11:49=E2=80=AFAM Kairui Song wro= te: > > From: Kairui Song > > This is only one caller now in page fault path, make the result return > argument mandatory. > > Signed-off-by: Kairui Song > --- > mm/swap_state.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > index 6f39aa8394f1..0433a2586c6d 100644 > --- a/mm/swap_state.c > +++ b/mm/swap_state.c > @@ -913,7 +913,6 @@ static struct page *swapin_no_readahead(swp_entry_t e= ntry, gfp_t gfp_mask, > struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask, > struct vm_fault *vmf, enum swap_cache_resul= t *result) > { > - enum swap_cache_result cache_result; > struct swap_info_struct *si; > struct mempolicy *mpol; > void *shadow =3D NULL; > @@ -928,29 +927,27 @@ struct page *swapin_readahead(swp_entry_t entry, gf= p_t gfp_mask, > > folio =3D swap_cache_get_folio(entry, vmf, &shadow); > if (folio) { > + *result =3D SWAP_CACHE_HIT; > page =3D folio_file_page(folio, swp_offset(entry)); > - cache_result =3D SWAP_CACHE_HIT; > goto done; > } > > mpol =3D get_vma_policy(vmf->vma, vmf->address, 0, &ilx); > if (swap_use_no_readahead(si, swp_offset(entry))) { > + *result =3D SWAP_CACHE_BYPASS; Each of this "*result" will compile into memory store instructions. The compiler most likely can't optimize and combine them together because the store can cause segfault from the compiler's point of view. The multiple local variable assignment can be compiled into a few registers assignment so it does not cost as much as multiple memory stores. > page =3D swapin_no_readahead(entry, gfp_mask, mpol, ilx, = vmf->vma->vm_mm); > - cache_result =3D SWAP_CACHE_BYPASS; > if (shadow) > workingset_refault(page_folio(page), shadow); > - } else if (swap_use_vma_readahead(si)) { > - page =3D swap_vma_readahead(entry, gfp_mask, mpol, ilx, v= mf); > - cache_result =3D SWAP_CACHE_MISS; > } else { > - page =3D swap_cluster_readahead(entry, gfp_mask, mpol, il= x); > - cache_result =3D SWAP_CACHE_MISS; > + *result =3D SWAP_CACHE_MISS; > + if (swap_use_vma_readahead(si)) > + page =3D swap_vma_readahead(entry, gfp_mask, mpol= , ilx, vmf); > + else > + page =3D swap_cluster_readahead(entry, gfp_mask, = mpol, ilx); I recall you introduce or heavy modify this function in previous patch befo= re. Consider combine some of the patch and present the final version sooner. >From the reviewing point of view, don't need to review so many internal version which get over written any way. > } > mpol_cond_put(mpol); > done: > put_swap_device(si); > - if (result) > - *result =3D cache_result; The original version with check and assign it at one place is better. Safer and produce better code. Chris