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

[Yosry Ahmed]

> > > > > @@ -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.

>
> >
> > >
> > > 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.

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.