On Wed, Jan 29, 2025 at 2:08 AM Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> wrote:
>
> Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()")
> skips charging any zswap entries when it failed to zswap the entire
> folio.
>
> However, when some base pages are zswapped but it failed to zswap
> the entire folio, the zswap operation is rolled back.
> When freeing zswap entries for those pages, zswap_entry_free() uncharges
> the zswap entries that were not previously charged, causing zswap charging
> to become inconsistent.
>
> This inconsistency triggers two warnings with following steps:
> # On a machine with 64GiB of RAM and 36GiB of zswap
> $ stress-ng --bigheap 2 # wait until the OOM-killer kills stress-ng
> $ sudo reboot
>
> The two warnings are:
> in mm/memcontrol.c:163, function obj_cgroup_release():
> WARN_ON_ONCE(nr_bytes & (PAGE_SIZE - 1));
>
> in mm/page_counter.c:60, function page_counter_cancel():
> if (WARN_ONCE(new < 0, "page_counter underflow: %ld nr_pages=%lu\n",
> new, nr_pages))
>
> zswap_stored_pages also becomes inconsistent in the same way.
Nice catch haha.
>
> As suggested by Kanchana, increment zswap_stored_pages and charge zswap
> entries within zswap_store_page() when it succeeds. This way,
> zswap_entry_free() will decrement the counter and uncharge the entries
> when it failed to zswap the entire folio.
>
> While this could potentially be optimized by batching objcg charging
> and incrementing the counter, let's focus on fixing the bug this time
> and leave the optimization for later after some evaluation.
>
> After resolving the inconsistency, the warnings disappear.
>
> Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()")
> Cc: stable@xxxxxxxxxxxxxxx
> Co-developed-by: Kanchana P Sridhar <kanchana.p.sridhar@xxxxxxxxx>
> Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@xxxxxxxxx>
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx>
With your fixlet applied:
Acked-by: Nhat Pham <nphamcs@xxxxxxxxx>
