On Tue, Sep 24, 2024 at 2:38 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> 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...?
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 :(
