Re: [PATCH v1 5/8] mm: zswap: Store the folio's node id in the zswap_entry.

[Yosry Ahmed]

On Thu, Oct 15, 2026 at 3:45 PM Kanchana P. Sridhar
<kanchanapsridhar2026@xxxxxxxxx> wrote:
>
> On Thu, Oct 15, 2026 at 2:39 PM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
> >
> > > > > > > @@ -1449,9 +1446,10 @@ static bool zswap_store_page(struct folio *folio, long index,
> > > > > > >  {
> > > > > > >       swp_entry_t page_swpentry = folio_swap_entry(folio, index);
> > > > > > >       struct zswap_entry *entry, *old;
> > > > > > > +     int nid = folio_nid(folio);
> > > > > > >
> > > > > > >       /* allocate entry */
> > > > > > > -     entry = zswap_entry_cache_alloc(GFP_KERNEL, folio_nid(folio));
> > > > > > > +     entry = zswap_entry_cache_alloc(GFP_KERNEL, nid);
> > > > > >
> > > > > > We don't need to allocate the entry on the same node anymore, should we
> > > > > > drop this and let the slab allocator handle it as it sees fit?
> > > > >
> > > > > Was this a recent change? I see the folio_nid(folio) on my version of
> > > > > mm-unstable as of 10-3-2026. Let me check, and yes, allowing the slab
> > > > > allocated handle this is probably a good idea.
> > > >
> > > > We allocate the entry on the same node to avoid having to store nid in
> > > > the zswap entry (see entry_to_nid()). If we store it, then there's no
> > > > need to allocate on the node.
> > >
> > > With patch 0006, we try to bulk allocate all entries for the batch.
> > > Only if this fails, we call zswap_entry_cache_alloc(gfp, nid). So
> > > patch 0006 addresses this already, and the nid is used on in the
> > > fallback case.
> >
> > If we are storing the nid in the zswap entry, there is no need for
> > zswap_entry_cache_alloc() to use the nid anymore.
>
> As of now, zswap_entry_cache_alloc() uses the nid for
> kmem_cache_alloc_node(). Is the suggestion to change this to be
> kmem_cache_alloc()?

Yes. I don't think we need to use kmem_cache_alloc_node() if we store
nid in zswap_entry.

>
> >
> > >
> > > >
> > > > >
> > > > > Along the same lines, I have been working on replacing zsmalloc with
> > > > > direct SLAB allocations for the zswap compressed data storage and see
> > > > > very promising results: memory savings and latency improvements. I
> > > > > will submit an RFC shortly.
> > > >
> > > > Keep in mind that zsmalloc is movable memory, while slab memory isn't.
> > > > Zsmalloc may also be less likely to make a new allocation in the store
> > > > path. There are other considerations beyond memory overhead and
> > > > latency here.
> > >
> > > I realize this means we trade off page migration for writeback for the
> > > pages representing zswap compressed data. This might not necessarily
> > > be a bad thing for 2 main reasons, the way I see it in this specific
> > > context:
> > >
> > > 1) Page migration would only be transferring a memory pressure problem
> > > elsewhere. Not sure how well this would work with memcgs or
> > > containers. At some point someone would need to do writeback.
> > > 2) I don't believe there is a zswap specific page migration flow in
> > > the kernel other than zsmalloc compaction. IOW, memory tiering
> > > components like CXL are handled directly in the reclaim path. From
> > > what I understand, things wouldn't break with slab memory for zswap
> > > compressed storage, but I could be wrong. I hope to include the
> > > architecture level considerations/trade-offs in the RFC.
> >
> > Zsmalloc memory is currently movable, so compaction can move it around
> > to free contiguous blocks (e.g. for THP allocations). Slab memory is
> > not movable. It can be reclaimable, but writing back memory from zswap
> > when we are not under memory pressure is also bad.
>
> All good points, thanks. I agree, writing back memory from zswap when
> not under memory pressure is bad, however I have not seen any
> writebacks in my experiments with usemem at 60% over-commit and with
> 64kB mTHP.

We can discuss further when you make your posting :)

>
> I am running these experiments in a memory constrained environment:
>
> QEMU VM: 7G RAM
> Swap: 3G
> cgroup memory.high: 3G
> 4 usemem processes want to allocate 1200M each. To make the experiment
> even more stringent and representative of actual compression datasets,
> each usemem process reads the Silesia dataset from a
> pre-fork-allocated buffer into the 1200M. This causes realistic
> compression payloads in usemem.
>
> >
> > Note that compressed memory can be very large compared to slab
> > allocations (or kernel allocations in general), so it can increase
> > unmovable (or reclaimable but not movable) memory significantly,
> > causing fragmentation.
>
> It took me a while to experiment with compressed size bins, until I
> settled on a compromise between memory wasted per slab and zswap
> latency in finding the correct size bin to minimize fragmentation. I
> have been gathering extensive SLUB stats which indicate:
>
> 1) Even under peak zswap compressed quota ("pool") utilization across
> 10 consecutive iterations, SLUB comes out ahead in less overall memory
> overhead.
>
> 2) At steady state (when 10 iterations of the workload are done), most
> of the slabs have been returned to the Buddy allocator, except for 3M
> worth of slab setup per compressed size bin (0 objects in use): this
> is as compared to 32K of zsmalloc slub metadata that remains at steady
> state.
>
> I learned some really interesting things from these experiments, which
> pose questions about the memory penalty due to latency that zsmalloc
> adds to zswap compress/decompress paths vs. "internal fragmentation".
> I expect to quantify this with data from my experimental setup.