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 541BDC54E76 for ; Mon, 20 Nov 2023 11:17:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E980C6B0443; Mon, 20 Nov 2023 06:17:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E46936B0446; Mon, 20 Nov 2023 06:17:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CE7A06B0448; Mon, 20 Nov 2023 06:17:46 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id BE4E96B0443 for ; Mon, 20 Nov 2023 06:17:46 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 841A280209 for ; Mon, 20 Nov 2023 11:17:46 +0000 (UTC) X-FDA: 81478082532.12.1784506 Received: from mail-lj1-f169.google.com (mail-lj1-f169.google.com [209.85.208.169]) by imf28.hostedemail.com (Postfix) with ESMTP id A987EC0014 for ; Mon, 20 Nov 2023 11:17:44 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aRIotgZy; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.169 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=1700479064; 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=HlAzsIeNdJrrb0PCgVXBwtQkX4AXQb5hB3gxR/EMw3M=; b=p6LzDpV5McnPWTB9G5swPdHM2Bj30AwDdQpHww0aGL/P7U64VjNt7ESGFacgc2OFFYMxXP pmAIjcuWQL1FrdyShPDtFSLfrB0MvUx4mrf+FljrJc0R2EvSmdknXorktM05d2y4TYxroD oN82ATE0RguBCQINvkD8cxIizrTHVaU= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aRIotgZy; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.169 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1700479064; a=rsa-sha256; cv=none; b=JKWI9OaDrJqmrpRb/1fzAll11qrkkv4FYCGzhKrn37ViZxMJPdmgsrO9QD5aMbKziE6NzJ M9Pzzd8c0N7GOYnCsV9y1ixeloZXn+DIGLK22Oe6vX4EOCG9oQ195GzDxsEzPT9csYvQf+ SJDgliSvYkUi4Q16DCgRTF0nDLtgyF8= Received: by mail-lj1-f169.google.com with SMTP id 38308e7fff4ca-2c503da4fd6so53566961fa.1 for ; Mon, 20 Nov 2023 03:17:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700479063; x=1701083863; 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=HlAzsIeNdJrrb0PCgVXBwtQkX4AXQb5hB3gxR/EMw3M=; b=aRIotgZyh31i4Pt5USpdjrI/bbPXmcuupcU/q9jTAG28i8MxhP5W6D3FOE/7LdKPwE 6u5T1FZjG6GscENgeYZ5pfB+JVAeFog7i4TbTEKPrYPovkcnm6yFVAT/wLHXMuwGvmOD tPdkYHGGj22j3TvvqG/2LjOC/VZQ2hAhmoVWHzrlwenYFdmsKY8xuAqyuEAGNmngBIGr k/zxqv65uECQut4sm3mEvHgXvDArKL8DHmMIbX5wDQq+1aYnZx3qbCuxaRWEJdRDkN8J xjIcmT3l+xilYLi1bPdLauQoWZLkCMw49KgYPMlXF1zauLfoPWokUcIWUfNAyqsv/qPY QvBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700479063; x=1701083863; 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=HlAzsIeNdJrrb0PCgVXBwtQkX4AXQb5hB3gxR/EMw3M=; b=v8u3+/Xkc2RIrgH3yo+J3hKiUcvQhV6dRwT6pPebFbmagWEl0W2r4cOqzyW3OHS73P OTMPRx0YbF7gEY6WCtsBC5xucFcWZkE2a1ReBP3s5i3ZSZQUydXVq5es021ciuKYjtd8 vIPF/JH2cChWkXepCZoI9HqoaeQObN8qjwbY5pwCFTM6FCMui2qKfOwJrwbQRZS2DEtr VSKTjxLbltAMY319y4xbaFDWNxpfAgQ7J/TYJjhXgDLZ33vFabHpncjUoworzglP9zeR 79dny1gI0qFWd+59xOENTQzw5vroJSTB0eQL6sHDj5+z6fHQ1hNTSXQjjnNaRAAEJipI zqBw== X-Gm-Message-State: AOJu0Yx5fAAw9BPsVMXSxa7gLAJmT5ihS0/TvJI4XqIfmyhTjdjsjPMp KdhvKDrbzK3YhjWePGJMFvwEuJpZdDh62p3ky1A= X-Google-Smtp-Source: AGHT+IGROfOVRQGMAWNU064ZJvUcxyA8rNdh/U5bUHgKrIdtR5uJKxgBEdgG1lmgfUYHqP6WcQmWUL3CwfpOdnAodo0= X-Received: by 2002:a2e:9008:0:b0:2c8:7176:1727 with SMTP id h8-20020a2e9008000000b002c871761727mr4568048ljg.5.1700479062869; Mon, 20 Nov 2023 03:17:42 -0800 (PST) MIME-Version: 1.0 References: <20231119194740.94101-1-ryncsn@gmail.com> <20231119194740.94101-20-ryncsn@gmail.com> In-Reply-To: From: Kairui Song Date: Mon, 20 Nov 2023 19:17:24 +0800 Message-ID: Subject: Re: [PATCH 19/24] shmem, swap: refactor error check on OOM or race 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-Server: rspam09 X-Rspamd-Queue-Id: A987EC0014 X-Stat-Signature: euu595qstqdkkf1fyugi8jqf5ikxah9i X-Rspam-User: X-HE-Tag: 1700479064-389338 X-HE-Meta: U2FsdGVkX19B/yjoOX0LMCUPSEJ4WSKH+y30jA2S3ovWQM0W8SqFIdJR3okr9DxLNrjNnc7C8GBxYuWAfnh6jU+hBRK/bnd4V+p45kE7Ldkrlm/ahGZZRMIxFYK4wZ8A/Z3V7gFhG8so7NtzBBzsF3jouj6riQdXLFwauwxsSukKkGnb6bSw0q6arMF08Eb+eKSdLTr/SbWYkcH/oF2BVcvAtzO3dzx6FVXu2BltkB5Yb6mnyqkAjtka10dBKHAFrp3QOvwECWD0z0TfbAyWkRFaNRNk08e77UX/dWA58Y1wyuszKmUNzxr59DoRVri101qnVXUHDr4Pnw3AjXg4SMqG+x4HGnyssRaFGTNBQwRSzLkaaSqe/MS+7Xv79jJr4+MVEstisDfcHbceRgr/IxCP+ZHpqgdWqwxCnxPNPlWzshqDRWlIrtw/eRPYjned2Xfwq4HV6ZYuZodgwGp7X080PWgH6mcQYr0ehcPECBBqWsGbbhXlQ8jrT6nYC7OWKJyfgdjS/919riGmsgeUvNB+a/12swF3Ir6y1oNOsv1L6Jx80aHxMvuCFzeC8p8O2d4PvX8FbxdsfjsW4t7o3vgdgIvr5kqsIPtuVCh3OgthEXh7y+zQ5dQU+CZx+RBRlm5m94AuNFkvLKIliqq1cZ06EfBlJIlZCBHkLuTuQpyEeU8zXZHby4VDZHZdalviRO3NIzD0Ftj7VDjIc77HRalssc6D//ddsucvHIuYAIQyTUZoLnEBJiQSd5ixwUNZTRglylWMhc/nCiVW7A34m94uuvS9om/KapXo7PAH4hZ+ImpzkKDkvCkf7kLUx3CMkCCOVrCpGErzC+eWtGWW5ojIBTV8n7X643sTHA+qXjf83JBhhQxfvYotmfjPOG7uXT7JYZbaCp6N6RQti+AdTUZjH9G/rqyi6vsrI0FJVT5wz+F5JvddEMW+PBqeqErXMg3UsCsPl08GxWYeKDZ ZAkA/U8v qX//kLT6avPSQR64aobSiKFKrKeqJ/7ftCxAe+cmq4D/aeIQ4TZxQva3NvnfsFEdWpnQzPKkAHZFd4OhrfYgo3PJvOroISw6zOQSb07SE/elz9bRLsy/C7bxiX0LCnymLtLS9q+3mKj60s7vY7zKzrW8jZ2YaHItl5NpMF5cJLB9dshuayPoAdDKGe9Pt6G1cNcuac/1KlIsUPKeD3hs9o/MFtTfBs6e0NXAV5I5bjF4EtNl7vMf7Q4KhjEzVIxTbMRudkYOpdfthq4Ei7v7m8cXG3eFYO0zyvHkiv2EjclwDOFpVooNi1jSTKrEbdZj/4PIfZVX8ndSkmSjzIQvcLJS5NAglLx/bCdbHLigcmbPQoFftUX+H4rVPZeYA02xLZsjDFYnmvOaY2QhGsWRSc29uk42rnL9F3FfOUvAc7i/T5pDkOGKGITVe5anDYyAc29Pdrt0KRQusJk/PyyXdzf7CIkk3cI8j+YcP+N9hVGwnyjdHj3LxPSeWcA== 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=8820=E6=97=A5= =E5=91=A8=E4=B8=80 15:12=E5=86=99=E9=81=93=EF=BC=9A > > Hi Kairui, > > On Sun, Nov 19, 2023 at 11:49=E2=80=AFAM Kairui Song w= rote: > > > > From: Kairui Song > > > > It should always check if a swap entry is already swaped in on error, > > fix potential false error issue. > > > > Signed-off-by: Kairui Song > > --- > > mm/shmem.c | 10 ++++------ > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > index 81d129aa66d1..6154b5b68b8f 100644 > > --- a/mm/shmem.c > > +++ b/mm/shmem.c > > @@ -1857,13 +1857,11 @@ static int shmem_swapin_folio(struct inode *ino= de, pgoff_t index, > > page =3D swapin_page_non_fault(swap, gfp, mpol, ilx, fault_mm, = &result); > > mpol_cond_put(mpol); > > > > - if (PTR_ERR(page) =3D=3D -EBUSY) { > > - if (!shmem_confirm_swap(mapping, index, swap)) > > - return -EEXIST; > > Do you intentionally remove checking shmem_confirm_swap()? > I am not sure I am following. Hi, Chris, thanks for the review. This was also called under failed: label so I think there is no need to keep it here. > > > + if (IS_ERR_OR_NULL(page)) { > > + if (!page) > > + error =3D -ENOMEM; > > else > > - return -EINVAL; > > - } else if (!page) { > > - error =3D -ENOMEM; > > + error =3D -EINVAL; > > The resulting code is a bit hard to read in diff. In plan source it is li= ke: > > if (IS_ERR_OR_NULL(page)) { > if (!page) > error =3D -ENOMEM; > else > error =3D -EINVAL; > goto failed; > } else { > folio =3D page_folio(page); > if (fault_type && result !=3D SWAP_CACHE_HIT) { > *fault_type |=3D VM_FAULT_MAJOR; > count_vm_event(PGMAJFAULT); > count_memcg_event_mm(fault_mm, PGMAJFAULT); > } > } > > First of all, if the first always "goto failed", the second else is not n= eeded. > The whole else block can be flatten one indentation. Yes, thanks for the suggestion. > > if (IS_ERR_OR_NULL(page)) { > if (!page) > error =3D -ENOMEM; > else > error =3D -EINVAL; > goto failed; > } else { > > Can be rewrite as following with less indentation: > > if (!page) { > error =3D -ENOMEM; > goto failed; > } > if (IS_ERR(page)) { > error =3D -EINVAL; > goto failed; > } > /* else block */ > > Am I missing something and misreading your code? Your code looks cleaner :) This patch is trying to clean up the error path after the helper refactor, and your suggestions are very helpful, thanks! > > Chris