From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2655913D886 for ; Thu, 25 Jun 2026 00:14:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=209.85.128.47 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782346454; cv=pass; b=e5RISyig3yNNaPIax52CYcWi/ZfthG8BcgMDBZ/2kyvC4RQEveAMidb/kfzLMT5aVgN8haU1ExWSONVoGmPvuHgEmzapER3H1Yn5qOlPdDQdpEMRTRkasAZYTIPsYMtaNVVO+yZFwUTLuXI9ozQ2PjC2sYt3HZ8NM3HJBqUyCU0= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782346454; c=relaxed/simple; bh=rbwxD9NvlBeLm5utdKHPN9vWZkwTv/BkkG91wENKrrI=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=ODDb909gX8UhfplLO/JtPNGdvqJKzyNDasVNy1JotGLH6fTmcqNl1dwkjQhKFDcPH/noAhs9eqFEwYhmSxYEFvGUecgtGR/vlqTRmheozKoEgRzgq7o8vm4vJuosEy9EEYbJBD7xj2uHtccVzKNd+4crjidadLOQWtLwPd111bc= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=VUaVg9gt; arc=pass smtp.client-ip=209.85.128.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="VUaVg9gt" Received: by mail-wm1-f47.google.com with SMTP id 5b1f17b1804b1-4924593f45dso18764095e9.1 for ; Wed, 24 Jun 2026 17:14:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1782346450; cv=none; d=google.com; s=arc-20260327; b=HD/5JEl+sjxBu76LAitHNZ+K488O+WV81gNVsF7CRotgz1h+y+bRU6uUXbdT3fP+Mk /HfYPEMlvL7jdt09fQWC65MbinjOPXl5JiY3GfXgu8SFF3boI4n/Jv7f7i9Vo+9km3t2 MVgHpPBlMKO/Y0SzGFcRW/DttmYd18F+hXuxfnAPpSQUWV9SLh70Xv5G1loqpUzvGMZv wPOQ7oxJSfh64ok5R635hQ98NzDHaMcwMuysPRpBXvCvwAghnWS4OhNTVHU2R0ddunq1 YFffb2vd1+sQ6hPoK9R0FmXk2OiCIbzAgJXe/AwjsI8uITpdgW5tcA/CdAqOAdlUXjaz lqMw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=zQUU4QEwAFRoM46ZCAt+65nFqlnbcp63bvZEbEWuj4k=; fh=4TZJke9HLmGmfDEeGtbVwC1eo80YEzVd8Oe6ude7DgA=; b=rlF116dhkPIZyNkJfkZo5ukLCxgz4E5+S1RXAbht4QvV79tACp7unBv+Y5Fx9F4lW9 6PUJ3TNTeEweJmP81yk5Kvr/atC4ss6W2p90TTRZNJlDn9YYlVaDKWLuJkkgVDh2EjoJ u/wp5dGF4h6Z/2EaCN/3oJSwLK4JoN8rbHbhWNg4JqXlx8TD/az4qlUkgbVmPosoGvLu AkCj3RNbxEl31hBBrDarARenDIWE3k6fDn8VkoDsiRomFiAEuovVscPRXBTf+afSfd5p meRutvenNKd4BGtOdcOfITvcHdgTyekC6T7zWwWnvgcbqXftl8rgMgGNli0iBvjjp2st GDXA==; darn=vger.kernel.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782346450; x=1782951250; darn=vger.kernel.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=zQUU4QEwAFRoM46ZCAt+65nFqlnbcp63bvZEbEWuj4k=; b=VUaVg9gtomuEqay9V35r3TsLS30xv7VoMAoQB/X1stelj10FjfNakESVqCwg1P56Dx UFLFyIKwZu5YWLPJ0S31mVaLEqN8KOfoK7EwC+hoDCuilO7/wXzpp/KMDh1BfVgpnOwF QzxEZN49PxZ/vblgzYVZMj6RVyned4YKBJmlig0s0asP47zhmfF92MoVYxg0AboCWmqr nfhWqRnKJk5SbsU1EXHDuVMFSCAHw+H8T/s3cL0lGjAgDJRx5/u08cixdDo/jVn+lfaZ avdGDUNb5gJs9B3JpAoCJqQxrSZ1AxAL1qQB/XVd9JGtCjwjYX+gYiQo4T3Kmv1XCPyz 8Fcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782346450; x=1782951250; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=zQUU4QEwAFRoM46ZCAt+65nFqlnbcp63bvZEbEWuj4k=; b=CDgIRVQr00pU8vnULkJmcYThtVV2MhTL/xf25gKLaLVl9JHafDExXTKQaTyeJyH9TH x+JrWwaffJ+Kw431Atfcd6ntAVa3eZTtv+L1bmHziilh9HmvCnEFo34XK9EwAPQzRiNp KjpN5b2uzNbi7efD43Cw0gdnjPIxifLa1xwvMD0k+ZyFQ+TVcowOPW+FL4yyCqfTrsCB JZnWzc+TybYXQyUy9atD4SFEuL2JPTtCEye7kii7+lAa2O63DTY5FaOXX1pkehWtMpdl rw+/bh2hYzUyvY8SQ482KaKgveN4p99nngJNgDu7OdpKyuPWcBFuBl/TkI2Vjo6qf/8p YM5w== X-Forwarded-Encrypted: i=1; AFNElJ/ha+wW1ex8Jx2jM6S8M9sAPWzOdoOEMfYfUr+I5nLwqKqXYR/NLs7HZTAsWySZA4VywxHCs6zsfXJ6SdA=@vger.kernel.org X-Gm-Message-State: AOJu0YzeeON2tpIOv6+V1YYc4Jl/IUb4OW7fJxGz2zS6t6LlefcTFLx2 AaKyCR8f30MjguDV/doSHtmrUNqaVXqlSN+WZDq6YeN/Fd7vpeCXMPL4r7oIraEoP39/FhXhn49 5RK2F8yrGzsO63rVvXNUe9wFR0JYUpSA= X-Gm-Gg: AfdE7ckOSS+rjYrFtFhs2wSc/zrhByh0d6VwqcGzD4AvsagGZYrQonEHYZBJcygodE6 f/DghcVAVq4EJbXgtobOtOqIKHnu1Zwg6QiB2ReZwv4uXhpKu63PHdba336lgYKYArh0yaXgU34 H1Yiyx6rJd0F3krTLSGPOTaQxcrbdgIL5Hd6HO1Dn/oSwpsG0Hwj78H6f64on0fu5RLSAxJ4ykE 3CsQcodEolqWqsb3ATcnRFXS0Z5hSZP7xr1mKmbMCmrkAK2vz2NduqMxu0IJq++ePPilVN+Be81 gCNPAl4BdlMy2AWn2YDESlAv758GjLXYRjA+K3ea+UNYgyufAQTd128= X-Received: by 2002:a05:600c:a46:b0:492:604b:c77b with SMTP id 5b1f17b1804b1-49266885dcfmr780335e9.26.1782346450038; Wed, 24 Jun 2026 17:14:10 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20260612193738.2183968-1-nphamcs@gmail.com> <20260612193738.2183968-4-nphamcs@gmail.com> In-Reply-To: From: Nhat Pham Date: Wed, 24 Jun 2026 17:13:58 -0700 X-Gm-Features: AVVi8CepHOibpPdukelNQnwEdTBgwRTdcmtt33fMSa9KmP8wKp2yKEwbQKXWFqM Message-ID: Subject: Re: [RFC PATCH v2 3/7] mm, swap: support physical swap as a vswap backend To: Yosry Ahmed Cc: akpm@linux-foundation.org, chrisl@kernel.org, kasong@tencent.com, hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev, shakeel.butt@linux.dev, david@kernel.org, muchun.song@linux.dev, shikemeng@huaweicloud.com, baoquan.he@linux.dev, baohua@kernel.org, youngjun.park@lge.com, chengming.zhou@linux.dev, ljs@kernel.org, liam@infradead.org, vbabka@kernel.org, rppt@kernel.org, surenb@google.com, qi.zheng@linux.dev, axelrasmussen@google.com, yuanchu@google.com, weixugc@google.com, riel@surriel.com, gourry@gourry.net, haowenchao22@gmail.com, kernel-team@meta.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Jun 23, 2026 at 12:02=E2=80=AFPM Yosry Ahmed wro= te: > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 466f8a182716..5daff7a25f67 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -993,6 +993,7 @@ static int zswap_writeback_entry(struct zswap_entry= *entry, > > struct folio *folio; > > struct mempolicy *mpol; > > struct swap_info_struct *si; > > + swp_entry_t phys =3D {}; > > int ret =3D 0; > > > > /* try to allocate swap cache folio */ > > @@ -1000,16 +1001,6 @@ static int zswap_writeback_entry(struct zswap_en= try *entry, > > if (!si) > > return -EEXIST; > > > > - /* > > - * Vswap entries have no physical backing - writeback would fai= l > > - * and SIGBUS the caller. Bail before we waste a swap-cache fol= io > > - * allocation. > > - */ > > - if (si->flags & SWP_VSWAP) { > > - put_swap_device(si); > > - return -EINVAL; > > - } > > - > > mpol =3D get_task_policy(current); > > folio =3D swap_cache_alloc_folio(swpentry, GFP_KERNEL, BIT(0), = NULL, mpol, > > NO_INTERLEAVE_INDEX); > > @@ -1028,40 +1019,78 @@ static int zswap_writeback_entry(struct zswap_e= ntry *entry, > > /* > > * folio is locked, and the swapcache is now secured against > > * concurrent swapping to and from the slot, and concurrent > > - * swapoff so we can safely dereference the zswap tree here. > > - * Verify that the swap entry hasn't been invalidated and recyc= led > > - * behind our backs, to avoid overwriting a new swap folio with > > - * old compressed data. Only when this is successful can the en= try > > - * be dereferenced. > > + * swapoff so we can safely dereference the zswap tree (or vswa= p > > + * vtable) here. Verify that the swap entry hasn't been > > + * invalidated and recycled behind our backs, to avoid overwrit= ing > > + * a new swap folio with old compressed data. Only when this is > > + * successful can the entry be dereferenced. > > */ > > - tree =3D swap_zswap_tree(swpentry); > > - if (entry !=3D xa_load(tree, offset)) { > > - ret =3D -ENOMEM; > > - goto out; > > + if (swap_is_vswap(si)) { > > + if (entry !=3D vswap_zswap_load(swpentry)) { > > + ret =3D -ENOMEM; > > + goto out; > > + } > > + /* > > + * Allocate physical backing BEFORE decompress - if it = fails, > > + * no wasted work. folio_realloc_swap sets vtable to PH= YS, > > + * overwriting ZSWAP - the old entry pointer is only he= ld > > + * by the caller now. > > + */ > > + phys =3D folio_realloc_swap(folio); > > + if (!phys.val) { > > + ret =3D -ENOMEM; > > + goto out; > > + } > > + } else { > > + tree =3D swap_zswap_tree(swpentry); > > + if (entry !=3D xa_load(tree, offset)) { > > + ret =3D -ENOMEM; > > + goto out; > > + } > > There's a lot of divergence in the code (in this patch and previous > ones). Seems like a lot of it is to do xarray operations vs vswap > operations. I wonder if we can abstract these into helpers, e.g. > zswap_tree_store(), zswap_tree_load(), etc. Maybe the name is not the > best, but you get the point :) How about zswap_entry_load() and zswap_entry_store()? :) > > Here we can then do zswap_tree_load() for both code paths and only the > folio_realloc_swap() needs to be different for vswap. We can do > similar cleanups for the load/store paths as well. > > > } > > > > if (!zswap_decompress(entry, folio)) { > > ret =3D -EIO; > > + /* > > + * For vswap: folio_realloc_swap already moved the entr= y > > + * out of the vtable. Restore it via vswap_zswap_store = so > > + * the entry stays tracked (and the just-allocated PHYS > > + * slot is freed). For non-vswap: entry is still in the > > + * zswap tree. > > + */ > > + if (swap_is_vswap(si) && phys.val) > > + vswap_zswap_store(swpentry, entry); > > Should this go in the cleanup path instead (i.e. in the 'out' label?). Ah, maybe if (ret =3D=3D -EIO &&)... > > > goto out; > > } > > > > - xa_erase(tree, offset); > > + if (!swap_is_vswap(si)) > > + xa_erase(tree, offset); > > Maybe this can also be abstracted into a helper, but I wonder what the > corresponding vswap operation would be. I think folio_realloc_swap() > will have already "erased" the zswap entry from vswap. Maybe have a Yup that's the right logic. We already change the backend to physical swap slot here, so there's no real "erase". > vswap helper that will only remove it if it's a zswap entry? We can > probably do a lockless check first to make it cheap? > > It's probably silly to do this, and maybe there's a better way. > Generally, I think the code would be easier to follow if we abstract > away the xarray vs. vswap stuff into helpers (where it's reasonable). I'm not entirely sure if its worth it either, yeah. Unlike load and store, erase seems a bit asymmetric in the sense that we only need to do it for non-vswap cases.