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

[Kanchana P. Sridhar]

On Thu, Oct 15, 2026 at 3:48 PM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> 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.

Got it. I will make this change.

>
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > 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 :)

Sounds great, thanks!

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