linux-kernel - Re: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in zswap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJD7tkaRbsc7Lq7ihfony7sGwyiUjioQVAi7gZoWGxFmc+nT2A@mail.gmail.com>
Date: Tue, 24 Sep 2024 17:52:45 -0700
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Nhat Pham <nphamcs@...il.com>
Cc: Kanchana P Sridhar <kanchana.p.sridhar@...el.com>, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, hannes@...xchg.org, chengming.zhou@...ux.dev, 
	usamaarif642@...il.com, shakeel.butt@...ux.dev, ryan.roberts@....com, 
	ying.huang@...el.com, 21cnbao@...il.com, akpm@...ux-foundation.org, 
	nanhai.zou@...el.com, wajdi.k.feghali@...el.com, vinodh.gopal@...el.com, 
	joshua.hahnjy@...il.com
Subject: Re: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in zswap_store().

On Tue, Sep 24, 2024 at 4:11 PM Nhat Pham <nphamcs@...il.com> wrote:
>
> On Tue, Sep 24, 2024 at 2:38 PM Yosry Ahmed <yosryahmed@...gle.com> wrote:
> >
> >
> > We can also do what we discussed before about double charging. The
> > pages that are being reclaimed are already charged, so technically we
> > don't need to charge them again. We can uncharge the difference
> > between compressed and uncompressed sizes after compression and call
> > it a day. This fixes the limit checking and the double charging in one
> > go.
> > I am a little bit nervous though about zswap uncharing the pages from
> > under reclaim, there are likely further accesses of the page memcg
> > after zswap. Maybe we can plumb the info back to reclaim or set a flag
> > on the page to avoid uncharging it when it's freed.
>
> Hmm this is just for memory usage charging, no? The problem here is
> the zswap usage (zswap.current), and its relation to the limit.
>
> One thing we can do is check the zswap usage against the limit for
> every subpage, but that's likely expensive...?

Ah yes, I totally missed this.

>
> With the new atomic counters Joshua is working on, we can
> check-and-charge at the same time, after we have compressed the whole
> large folio, like this:
>
> for (memcg = original_memcg; !mem_cgroup_is_root(memcg);
>      memcg = parent_mem_cgroup(memcg));
>      old_usage = atomic_read(&memcg->zswap);
>
>      do {
>         new_usage = old_usage + size;
>         if (new_usage > limit) {
>            /* undo charging of descendants, then return false */
>         }
>       } while (!atomic_try_cmpxchg(&memcg->zswap, old_usage, new_usage))
> }
>
> But I don't know what we can do in the current design. I gave it some
> more thought, and even if we only check after we know the size, we can
> still potentially overshoot the limit :(

Yeah it's difficult because if we check the limit before compressing,
we have to estimate the compressed size or check using the
uncompressed size. If we wait until after compression we will either
overshoot the limit or free the compressed page and fallback to swap.

Maybe a good compromise is to do the check before compression with an
estimate based on historical compression ratio, and then do the actual
charging after the compression and allow overshooting, hopefully it's
not too much if our estimate is good. We can also improve this later
by adding a backoff mechanism where we make more conservative
estimates the more we overshoot the limit.