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 41230C61DF4 for ; Fri, 24 Nov 2023 08:15:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C7B076B0111; Fri, 24 Nov 2023 03:15:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C02926B011A; Fri, 24 Nov 2023 03:15:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A58296B011C; Fri, 24 Nov 2023 03:15:10 -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 8A2AE6B0111 for ; Fri, 24 Nov 2023 03:15:10 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 64DD11210D1 for ; Fri, 24 Nov 2023 08:15:10 +0000 (UTC) X-FDA: 81492137580.11.6284427 Received: from mail-lj1-f173.google.com (mail-lj1-f173.google.com [209.85.208.173]) by imf06.hostedemail.com (Postfix) with ESMTP id 8BF57180020 for ; Fri, 24 Nov 2023 08:15:08 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=K2xSRF+1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.173 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1700813708; 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=5EYwIkp2+JQBbwt8LYg5s9Pu8iHhOSMhe1Ch2bTUfS8=; b=hU2XQfv0tXd/6AiVp8ctl72pfPEULjvqJXBBNEcn6X8S1DCOzrCTjOunaDc4t4Mw+dSIOZ XLhYVa+IMKxlppm8gJu2Vu0+PlSFnUwf41rSWpwIhJ6TYSmid3Rpy/c1hxO+b9mlFP1ivA 4tiWhRTvT5D61uuPJnC2O/r29PXo1QY= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=K2xSRF+1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.173 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1700813708; a=rsa-sha256; cv=none; b=GZFtYdrWV7PSJPPxCzucp9KPm3Ksb+TNxcDBylxIRQhhWsxo3bO+UTg2/k41t9IleO5Dpa 8gBwdiQ26WdSgRJgZZcuwhreyByBuNYZxVxPbI7C7kZmtKHCWUWlaNxvK40CpWec3U3RB2 OZGal6lmqnweDDQZL2HYlpHVJrwSrH0= Received: by mail-lj1-f173.google.com with SMTP id 38308e7fff4ca-2c875207626so19778341fa.1 for ; Fri, 24 Nov 2023 00:15:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700813707; x=1701418507; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=5EYwIkp2+JQBbwt8LYg5s9Pu8iHhOSMhe1Ch2bTUfS8=; b=K2xSRF+1P7EnXEcgCJmf15MhfRoz9Pk8xeYxKWbnj/2k84vn0jlYx+9FqGH5vFbSM/ RZlcYcG0oylpnpGYMWT1z7UI3H3Oit6rj6ZwFhYNvfMbMgHV8jtUOOq18g3Y97uOydWM ZLs/mwD5ZrEk+EwVNWkCnGsPOs2jb8vg4AvXMagY1cyPObIyBcbx1K0zvt/mFCjfR12F QRSNPh2E2A3MYBYSlPeG+JeVOs4eSZBlxtm10ai4GOKsvcvdEcqcp3DB/bKsGXV0qifR r1kaqp85raLWGSL7kzZ9qafAwL6VPde2CUUwHnCTP3ZZOjmkTDCrpJrYi7FtqORdUdUY VpOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700813707; x=1701418507; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5EYwIkp2+JQBbwt8LYg5s9Pu8iHhOSMhe1Ch2bTUfS8=; b=GLq68Nksfm53ol6IsImAG3IsQinkcvnjcEOvOlkobLq0tsLLl0F9e2+lTuJAmfG0f3 KnHLu4az1SWev9ZnHQtzKWtUhW6K4t+TrDEcmbseeZv9nMXaVZ91nKN6bTUGg/nJmT4h L6fqPUAhPOZVFkoAMJeqEvG3NeRgfOGGISNbNoSLED+4BZ93Hs4YldXA/s8PgW6ppqdr CA2k9dR0wcmSOda/pL5+Qfr7uxhfhdPxzdD+GhInvgZ6orgvZ2idAtSdRykeAYFw7vzK jaMitD8SBCa/M4dOHVBaNWK4wraIDi/ornI/QCjN6epsYw0mxQ3OEPTeBnwrWwaKdTyh aIWg== X-Gm-Message-State: AOJu0YxNNXGmfYcNdwFto8gPV7TS1MMsQptpJXybfhgDDU5HaEtorElr wTNetLbUu82F10+OFYp26ppOsiDDkgsxaz7uPw8= X-Google-Smtp-Source: AGHT+IHuAjr2hg83x7hwu14c0h4KwHcHHqArvnEQR9FpmuSLSDWtPuPaAuIT+hAxfw1N2h1khf2ZD80b2NpE99GcMHk= X-Received: by 2002:a2e:90c2:0:b0:2c8:8024:3447 with SMTP id o2-20020a2e90c2000000b002c880243447mr1378942ljg.44.1700813706787; Fri, 24 Nov 2023 00:15:06 -0800 (PST) MIME-Version: 1.0 References: <20231119194740.94101-1-ryncsn@gmail.com> <20231119194740.94101-22-ryncsn@gmail.com> In-Reply-To: From: Kairui Song Date: Fri, 24 Nov 2023 16:14:49 +0800 Message-ID: Subject: Re: [PATCH 21/24] swap: make swapin_readahead result checking argument mandatory To: Chris Li 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-Rspamd-Queue-Id: 8BF57180020 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: im7bwcncpo5c53jpnipwwhx59rxmjb3x X-HE-Tag: 1700813708-49236 X-HE-Meta: U2FsdGVkX1/HZlF+Fl26bxQojJ7lS08BgWZSQnoz2jtjMnrA8sLOebSulk/ZaADr1JxXQRehg5GuLPfMkLOrfsNyo6boH22KKucIZA+dbEpVPhtjGTBQQWDsFC8ii+4o4LJ0tyFiWhhcHQSjwldTzvmLpEHLA/koDSN+37CqG81nAevfKqRWTsl1w1gdH4OdDQN+eJv0wAzdfrG59UfVGZEudGB2ggUilcgLmBnSW7VTA6TCobrcV2tRe1WpvJi+nzJB2SOemJ65D3QWKW86MqH/TgaLwaKJ0m8/U84GWWJ1e3JyC7L/FeLJ0M03w7Deu6K0quuV9+2HderkR//PmK26VcNzrOMnaarzNTvm4mT4f/+5Ejof00J8PIc9PKikNLfDpNnn4AE51m/oRAYNn8ZHIfzEUOaRJwKOsw003PY9MqNHTF20LzU49uzmqOq7sTmGrfMMTzKCEHuOSoyxCmIlCLAg1+/UsmTDk6pAStXzbbyTCxRgFCsHJgyA38VTDyVNz5/OAEFvzvooD9/SJ0Uj0htjJyqER0FdTuWZw0zs/Nnvl59TlgDhnnejGx2ZgBYkFcJxjPjfuheSCP8GZeG25UrXdMiwv3l0rR5mgs587i/C+jb9NsYm00E5PdJN1kX5KkN7eruOaQApv3H5tKqHtp/tXG8hj6HbBBua//GET07d4zFZiAqpr5he4z5pSE0DoDJ8bSTlDt9YYl38xt00bTQARvBMdvvAS4dnA3yp7Oyq+U+/sQ8Ff584ziMOW0HcN0X/+nD9ANlIWIK0G6wIQRfcoP5XX/wnBH+PK+RRPd9VxR5xp0A5H6kx7JA+2IGTnIxC6TPSYHJGvDobZ2FVa8iOS/zuyLBQ4iMo4GKjPq0LqW8LK8o/wkkLbI8OtIqRNyLXDaN6eH9XdSneNN2U+qyG/Vvdf4vjcYfl7A8SHAgjzC4bNOJIXTJYNUQIL5SAJvZ54B38eZAnrz9 JIP/0XDu AOOf2zzsOy2/qSaAs5KjPHNdKTSkkAdknIFZMz5AwZfcPfvPH6WsdBakdG2ObsN0e30BYDCQIzUw57cF6JKiiB3szfiVzL103vhs+3K7Xfp6nuccQd/4Hfu6NYhgOTxZGyKbda7qJil5FbLRSIOyr6E+4bsKsWyRytucV0dC/lvu2GH0TXbjdhrGPXQPOz4u+Q657xXzaWzIIRdQWT1q4GYDjTO5ER5taSO/fdWE7qFuHc9V48/8BmnBRj3zpfpoyAb9ve3QZQ0rRHpw/PO9q11PqOCbqvHbZ1w/INB0zHzJ+Bp8pRvQp2Xf97aXoCs/X3mAgoUwJx1ggUUNfaZiNh9SzrDFInhc+gVA9O6KcZ5PcMk22yj2xX4b/TCRLD0OfzBjVp+YzmFXZXXN7kGNPwsrgjzSUlfHX2Hogv9er6FlPQwtsDaYS6Uo8Q5WwwTBDVotWQ6OJ8uLRrpysKSTBeGzvyJsH+NXhgNKMGEmXJnsQPhWoapCt6djBd3TGnhAyFAwGw7PailWWCrX39joaGN/L7UfrIEg5avuV 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: Chris Li =E4=BA=8E2023=E5=B9=B411=E6=9C=8822=E6=97=A5= =E5=91=A8=E4=B8=89 13:18=E5=86=99=E9=81=93=EF=BC=9A > > On Sun, Nov 19, 2023 at 11:49=E2=80=AFAM Kairui Song w= rote: > > > > 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= entry, gfp_t gfp_mask, > > struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask, > > struct vm_fault *vmf, enum swap_cache_res= ult *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, = gfp_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,= vmf); > > - cache_result =3D SWAP_CACHE_MISS; > > } else { > > - page =3D swap_cluster_readahead(entry, gfp_mask, mpol, = ilx); > > - 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, mp= ol, 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 be= fore. > 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. Yes, that's less error-prone indeed, saving a "if" seems doesn't worth all the potential trouble, will drop this.