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 F2EDFC71157 for ; Wed, 18 Jun 2025 03:07:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7853C6B0088; Tue, 17 Jun 2025 23:07:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 75CF96B0089; Tue, 17 Jun 2025 23:07:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 69A506B008A; Tue, 17 Jun 2025 23:07:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 5CCA96B0088 for ; Tue, 17 Jun 2025 23:07:47 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id D483481ACF for ; Wed, 18 Jun 2025 03:07:46 +0000 (UTC) X-FDA: 83567036532.04.5538936 Received: from mail-lj1-f177.google.com (mail-lj1-f177.google.com [209.85.208.177]) by imf09.hostedemail.com (Postfix) with ESMTP id DE0A9140006 for ; Wed, 18 Jun 2025 03:07:44 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=IdXuYvxp; spf=pass (imf09.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.177 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1750216065; 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=e3zUICPhinkw3JOiRXbperRYmBshkcCwZWbIoyWDapw=; b=LmefHYKN55gwhLPXzMfznHu/lWoqZ/L8BgJFZS0ky46gCLx2tr0hiwrUWGxWYeYDXzVrD4 dlWrWS8JwMW99l2NhTjR+7izyVQHSGDByev9yxFA6Hg77m1AwdpHzCQENq2QAoO9GSwsnK kkJSW2AuiSIvjF2ZWoj1PFuYpwzAbQY= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=IdXuYvxp; spf=pass (imf09.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.177 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750216065; a=rsa-sha256; cv=none; b=Qoma4KG4+WXSzLew6VkVq/Zdpo2rHmfEL8rwBdNYl8IEAWLhM8TC09kyRUVIlvfvpluvsp B3LJAvjhS6wKAS8sUi4BbQoxdwBnZcKoZ3kw5HbHmyO65yRBQMlKG4Y7jwCA7ZTjDZGBnE wsErQGHTaYtKjnU1uqTEXfF3M0iy0YI= Received: by mail-lj1-f177.google.com with SMTP id 38308e7fff4ca-32925727810so54960181fa.0 for ; Tue, 17 Jun 2025 20:07:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1750216063; x=1750820863; 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=e3zUICPhinkw3JOiRXbperRYmBshkcCwZWbIoyWDapw=; b=IdXuYvxpVbtsyiFGNHt/Q1cS4OcJHZCMIdPw0xKz5w+Tbl1p+INz7G3S0msBM98gqI CWHaNQxhrIpjrDEJQK+rEmBJTfcohe6cmDRPjx9tMdrbBQAvHfIh+k40pyEPwSsFKKqe 6j0nn7umAhBM/B40T1W62muao2nqGhLccKS8Ap9AFAfW4LGl+v/0WJiPov/LREYgTAG/ UmzHRJoMOGa4OxcPKsBFol9wbiDPUCewfpA9Fnguw9b/svhZzHcDsMBeV2/vW2NisTa+ rynC+/najQuGvVSVKE7zuYPgh3MbLpL49R2wNxpzZXdEO2uCkjrFJ7Zn6zJXl33NeVpJ j6Og== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750216063; x=1750820863; 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=e3zUICPhinkw3JOiRXbperRYmBshkcCwZWbIoyWDapw=; b=JFfa3E9KJHccc5+yDZulyEKUQpaDztRZXkYVHTYWfjxrx+Eo5067A6jniLH9YfAvpA td1dSUpZY8LQFM4lGXFFqbKk617ADbHHNzA5zi9ly2DyM1HyBbJZzr1nP5wZTNgcPML0 QPIqMdfa6RfVGSCDFHRV1ZHERf5WYEU26IVa7nAsPcq4NikEFBtzV7m9m3gUH8Mj8V+T NQPNLq9027RKYrgv7IFA6Un7VL8LGT1YLywjqkwDodcSD6VDM04r17uDbTXk9MtioNyT MtKUSPn2enrKsnWxY/SKdgz1BNfHsWQqv47vu15OH/is6L5g7NOdnqeamRh3/UZAD1zA sJTg== X-Gm-Message-State: AOJu0YzDf1GW+xinUNS8apbvjH1PANTBmgS9m8lOffIX55DjzJeuIQDw JQeBzb3LwcU8NTzl+1HdceKjQ8CAUSkKdrWFn1xVUA5uZT5MHcVPVwEsr6D+o4aUg8ojAgByxJv jfSCa+7vCLysNF6ezyF7Wabu+aG3Wco4= X-Gm-Gg: ASbGnctjGwsdjHH2LgT8Z/Q7+V1vhSeF5EuPFHXWPUlyTHwX1xeqni5k0EbGkbQvApk sBPsu0PBcDEVX14/sTGB+3GBoFKTGcokz0y2jafd6B2O/LJ4nFUPjhtifySvtHXselv+CVHZPOQ 20zBoYND/m9Sm/LIGP1C/Zd729ivtKlHCi07Xjg9F7Ip4= X-Google-Smtp-Source: AGHT+IGew/TsNPIq+Nroxr/dllhfSRSorwJkVcKVlL7nROnf5CDMeRSk0OAOCw4BMEH+9FlTlm7krtYWlIejHsTFydg= X-Received: by 2002:a05:651c:b2a:b0:30b:a20b:6667 with SMTP id 38308e7fff4ca-32b4a41e9cfmr49284001fa.9.1750216062834; Tue, 17 Jun 2025 20:07:42 -0700 (PDT) MIME-Version: 1.0 References: <20250617183503.10527-1-ryncsn@gmail.com> <20250617183503.10527-3-ryncsn@gmail.com> <17bdc50c-1b2c-bb3b-f828-bd9ce93ea086@huaweicloud.com> In-Reply-To: <17bdc50c-1b2c-bb3b-f828-bd9ce93ea086@huaweicloud.com> From: Kairui Song Date: Wed, 18 Jun 2025 11:07:25 +0800 X-Gm-Features: AX0GCFuKCXNr4NMt0l97oNd1EFA7bv0HDzZgopeZgfTh4Zz3W_HMm72pCEe3P4o Message-ID: Subject: Re: [PATCH 2/4] mm/shmem, swap: avoid redundant Xarray lookup during swapin To: Kemeng Shi Cc: linux-mm@kvack.org, Andrew Morton , Hugh Dickins , Baolin Wang , Matthew Wilcox , Chris Li , Nhat Pham , Baoquan He , Barry Song , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: DE0A9140006 X-Stat-Signature: hpczm549sueic954gzjwandjdpemcjdx X-Rspam-User: X-Rspamd-Server: rspam04 X-HE-Tag: 1750216064-24694 X-HE-Meta: U2FsdGVkX18leYr4/ZvTARQeLtHa7RXLUn6VEHe+1luoQCE3g9j4zvDeldFW0vmVvRrRfejQFAt3G0mg15xr9waLHaq/Op3MgTlF1UDq42X70wts3o7MhxFmY5T/diwq7k2FIUgxvP2E94uAROv/xT2TST4m4ymITOvJG1GRKjiS0q/K0o2z3iIfdBsydUKw58TJfW/hFk2Iyqmx1lXAY+n8+cqhLNoLDkPTSkD+1UAQJqzkRIvQiAqzXy5yEmeApWDN2qCM2SfmlRWyhvmNpqZHBByinovDZq193Wv5bdd8qw2B9JTiVf0VxnejGAaB7O8IN6P1R4TYjkx69urt6EKbtArpA6bjlAVQ/DJSJLZGU/398S1W1ha6nyZrnjMSg8bwkRQxxWcD77hWYsAsvIEwU4MQfNO2BLaHA/cHpOH0dQGWDYbOyoHNpeWYl5rC+J1y6dnMgcaWJNTje3Wgwi2WuBfBiUT7uzGc8pqB1MFp5tBOae98Be+H9EIoY1mTmw8XStJJkfJWxtAHXZaiq4zwm8JI4HKX23xD1HIG3QjIzys1QOKNmtwuq80LGZb3VBsGYjyyHwtBJ7ohQ2aAcWASGqyIS3oAZCrWt0vsowlcxuhw7aeAams34AGjiHFNMMCVJquJ06eGMGZiYyW9aaKifKeuGhN9RAGRP2gDYRSHzB447UGXGIpQwfnIxQq59p3Cu3REjsi67wwysMfbBEoGg1t12vgX7DlXySK1NRpIGAwUD28EsEHQMWJ2ymRyxTEMJmqGO3Ar/xegxCnrzLIqgf2w+ydqrxUhhXAPH/9WiGWnxsT1mth+viWWTpA3CiH+Oheb3G7WZbm5ObUfkjbnSeB8QYrlR7YTmvteGBJaXfsRX6Q6Lc12O9ky2c94okCfzMoG83jTKLguC/rSZJ6N4nitkP4f5pwSsyIYudnSo3n9sNzoDVhjxoOv2l7H5RRNiwhebyGKqF2BMtJ SbW9ff95 z+bu0DOGYxXhpruUtsFXQ+8fJtUvYiSR1iw0n9Fjt+2qkyvNM86XnaNvkK9bo1b3V1qRvP/P4khoaZAgSZ00PsDowYA6VG+72tnLNIau8PpiPX0Wa/qGo38hRQSr9/CWtrk+C+GEz0sDOP0SR1mc6SskZoiakZoJPnntuMLB0FcjibmO1F1xfvier1MgZFmQITrnnT3C4qRMzpNuqkCN0aJVLK2IXsPWQMHXjsFrLV+Dux6DY5VJ6eQdS3Jkn1daQVsKTbb9m4tMsTrkQ4p8OUDqlCYainWANJBHZdddWRtmXNTWT6b/Ip2lKmHfMB6jieHozaysFXr6zTyWSHTxklAe+t2euD06iEm0wZK59zSbr3dUHCBExF/qqjw== 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, Jun 18, 2025 at 10:49=E2=80=AFAM Kemeng Shi wrote: > on 6/18/2025 2:35 AM, Kairui Song wrote: > > From: Kairui Song > > > > Currently shmem calls xa_get_order to get the swap radix entry order, > > requiring a full tree walk. This can be easily combined with the swap > > entry value checking (shmem_confirm_swap) to avoid the duplicated > > lookup, which should improve the performance. > > > > Signed-off-by: Kairui Song > > --- > > mm/shmem.c | 33 ++++++++++++++++++++++++--------- > > 1 file changed, 24 insertions(+), 9 deletions(-) > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > index 4e7ef343a29b..0ad49e57f736 100644 > > --- a/mm/shmem.c > > +++ b/mm/shmem.c > > @@ -505,15 +505,27 @@ static int shmem_replace_entry(struct address_spa= ce *mapping, > > > > /* > > * Sometimes, before we decide whether to proceed or to fail, we must = check > > - * that an entry was not already brought back from swap by a racing th= read. > > + * that an entry was not already brought back or split by a racing thr= ead. > > * > > * Checking folio is not enough: by the time a swapcache folio is lock= ed, it > > * might be reused, and again be swapcache, using the same swap as bef= ore. > > + * Returns the swap entry's order if it still presents, else returns -= 1. > > */ > > -static bool shmem_confirm_swap(struct address_space *mapping, > > - pgoff_t index, swp_entry_t swap) > > +static int shmem_swap_check_entry(struct address_space *mapping, pgoff= _t index, > > + swp_entry_t swap) > > { > > - return xa_load(&mapping->i_pages, index) =3D=3D swp_to_radix_entr= y(swap); > > + XA_STATE(xas, &mapping->i_pages, index); > > + int ret =3D -1; > > + void *entry; > > + > > + rcu_read_lock(); > > + do { > > + entry =3D xas_load(&xas); > > + if (entry =3D=3D swp_to_radix_entry(swap)) > > + ret =3D xas_get_order(&xas); > > + } while (xas_retry(&xas, entry)); > > + rcu_read_unlock(); > > + return ret; > > } > > > > /* > > @@ -2256,16 +2268,20 @@ static int shmem_swapin_folio(struct inode *ino= de, pgoff_t index, > > return -EIO; > > > > si =3D get_swap_device(swap); > > - if (!si) { > > - if (!shmem_confirm_swap(mapping, index, swap)) > > + order =3D shmem_swap_check_entry(mapping, index, swap); > > + if (unlikely(!si)) { > > + if (order < 0) > > return -EEXIST; > > else > > return -EINVAL; > > } > > + if (unlikely(order < 0)) { > > + put_swap_device(si); > > + return -EEXIST; > > + } > Can we re-arrange the code block as following: > order =3D shmem_swap_check_entry(mapping, index, swap); > if (unlikely(order < 0)) > return -EEXIST; > > si =3D get_swap_device(swap); > if (!si) { > return -EINVAL; > ... Hi, thanks for the suggestion. This may lead to a trivial higher chance of getting -EINVAL when it should return -EEXIST, leading to user space errors. For example if this CPU get interrupted after `order =3D shmem_swap_check_entry(mapping, index, swap);`, and another CPU swapoff-ed the device. Next, we get `si =3D NULL` here, but the entry is swapped in already, so it should return -EEXIST. Not -EINVAL. The chance is really low so it's kind of trivial, we can do a `goto failed` if got (!si) here, but it will make the logic under `failed:` more complex. So I'd prefer to not change the original behaviour, which looks more correct.