| Message ID | 20250128174938.2638-1-42.hyeyoo@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v6.12,hotfix] mm/zswap: fix inconsistent charging when zswap_store_page() fails | expand |
On Wed, Jan 29, 2025 at 02:49:38AM +0900, Hyeonggon Yoo wrote: > Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > mistakenly skipped charging any zswapped pages when a single call to > zswap_store_page() failed, even if some pages in the folio are > successfully stored in zswap. > > Making things worse, these not-charged pages are uncharged in > zswap_entry_free(), making zswap charging inconsistent. > > This inconsistency triggers two warnings when following these 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 > > 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)) > > Charge zswapped pages even if some pages of the folio are not zswapped. > After resolving the inconsistency, these warnings disappear. > > Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") This commit is in 6.13, not 6.12, so your subject line is a bit confusing :(
On 1/28/25 19:18, Hyeonggon Yoo wrote: > On Tue, Jan 28, 2025 at 6:14 PM Greg KH <gregkh@linuxfoundation.org> wrote: >> >> On Wed, Jan 29, 2025 at 02:49:38AM +0900, Hyeonggon Yoo wrote: >> > Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") >> > mistakenly skipped charging any zswapped pages when a single call to >> > zswap_store_page() failed, even if some pages in the folio are >> > successfully stored in zswap. >> > >> > Making things worse, these not-charged pages are uncharged in >> > zswap_entry_free(), making zswap charging inconsistent. >> > >> > This inconsistency triggers two warnings when following these 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 >> > >> > 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)) >> > >> > Charge zswapped pages even if some pages of the folio are not zswapped. >> > After resolving the inconsistency, these warnings disappear. >> > >> > Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") >> >> This commit is in 6.13, not 6.12, so your subject line is a bit >> confusing :( > > Oh, thanks for catching. Will fix it. > Also, I noticed I incorrectly described the problem. > > Will send v2 (for v6.13!) after adjusting them. I think we use e.g. "v6.13 hotfix" only while the stabilization of 6.13 is ongoing, to indicate the urgency. Now it's too late so it would only confuse stable maintainers, while the patch is not directly aimed at stable, but through mm to mainline and then stable backport as usual. So I think you can just use [PATCH mm-hotfixes] at this point. > Best, > Hyeonggon >
On Tue, Jan 28, 2025 at 5:50 PM Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote: > > Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > mistakenly skipped charging any zswapped pages when a single call to > zswap_store_page() failed, even if some pages in the folio are > successfully stored in zswap. > > Making things worse, these not-charged pages are uncharged in > zswap_entry_free(), making zswap charging inconsistent. > > This inconsistency triggers two warnings when following these 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 > > 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)) > > Charge zswapped pages even if some pages of the folio are not zswapped. > After resolving the inconsistency, these warnings disappear. > > Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > Cc: stable@vger.kernel.org > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > --- > mm/zswap.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 6504174fbc6a..92752cd05c75 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1568,20 +1568,20 @@ bool zswap_store(struct folio *folio) > > bytes = zswap_store_page(page, objcg, pool); > if (bytes < 0) > - goto put_pool; > + goto charge_zswap; > compressed_bytes += bytes; > } > > - if (objcg) { > - obj_cgroup_charge_zswap(objcg, compressed_bytes); > - count_objcg_events(objcg, ZSWPOUT, nr_pages); > - } > - > atomic_long_add(nr_pages, &zswap_stored_pages); > count_vm_events(ZSWPOUT, nr_pages); Wait, it also does not account for these events/counters. Will send v2. Sorry for the noise. If I missed anything else, please let me know. > ret = true; > > +charge_zswap: > + if (objcg) { > + obj_cgroup_charge_zswap(objcg, compressed_bytes); > + count_objcg_events(objcg, ZSWPOUT, nr_pages); > + } > put_pool: > zswap_pool_put(pool); > put_objcg: > -- > 2.47.1 >
On Tue, Jan 28, 2025 at 6:14 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Jan 29, 2025 at 02:49:38AM +0900, Hyeonggon Yoo wrote: > > Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > > mistakenly skipped charging any zswapped pages when a single call to > > zswap_store_page() failed, even if some pages in the folio are > > successfully stored in zswap. > > > > Making things worse, these not-charged pages are uncharged in > > zswap_entry_free(), making zswap charging inconsistent. > > > > This inconsistency triggers two warnings when following these 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 > > > > 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)) > > > > Charge zswapped pages even if some pages of the folio are not zswapped. > > After resolving the inconsistency, these warnings disappear. > > > > Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > > This commit is in 6.13, not 6.12, so your subject line is a bit > confusing :( Oh, thanks for catching. Will fix it. Also, I noticed I incorrectly described the problem. Will send v2 (for v6.13!) after adjusting them. Best, Hyeonggon
On Tue, Jan 28, 2025 at 6:42 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 1/28/25 19:18, Hyeonggon Yoo wrote: > > On Tue, Jan 28, 2025 at 6:14 PM Greg KH <gregkh@linuxfoundation.org> wrote: > >> > >> On Wed, Jan 29, 2025 at 02:49:38AM +0900, Hyeonggon Yoo wrote: > >> > Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > >> > mistakenly skipped charging any zswapped pages when a single call to > >> > zswap_store_page() failed, even if some pages in the folio are > >> > successfully stored in zswap. > >> > > >> > Making things worse, these not-charged pages are uncharged in > >> > zswap_entry_free(), making zswap charging inconsistent. > >> > > >> > This inconsistency triggers two warnings when following these 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 > >> > > >> > 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)) > >> > > >> > Charge zswapped pages even if some pages of the folio are not zswapped. > >> > After resolving the inconsistency, these warnings disappear. > >> > > >> > Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") > >> > >> This commit is in 6.13, not 6.12, so your subject line is a bit > >> confusing :( > > > > Oh, thanks for catching. Will fix it. > > Also, I noticed I incorrectly described the problem. > > > > Will send v2 (for v6.13!) after adjusting them. > > I think we use e.g. "v6.13 hotfix" only while the stabilization of 6.13 is > ongoing, to indicate the urgency. Yes. > Now it's too late so it would only confuse stable maintainers > while the patch is not directly aimed at stable > but through mm to mainline and then stable backport as usual. Right, that's what I'm trying to do. > So I think you can just use [PATCH mm-hotfixes] at this point. Adjusted the subject and sent v2. Thanks for the explanation, that makes sense to me! Best, Hyeonggon
diff --git a/mm/zswap.c b/mm/zswap.c index 6504174fbc6a..92752cd05c75 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1568,20 +1568,20 @@ bool zswap_store(struct folio *folio) bytes = zswap_store_page(page, objcg, pool); if (bytes < 0) - goto put_pool; + goto charge_zswap; compressed_bytes += bytes; } - if (objcg) { - obj_cgroup_charge_zswap(objcg, compressed_bytes); - count_objcg_events(objcg, ZSWPOUT, nr_pages); - } - atomic_long_add(nr_pages, &zswap_stored_pages); count_vm_events(ZSWPOUT, nr_pages); ret = true; +charge_zswap: + if (objcg) { + obj_cgroup_charge_zswap(objcg, compressed_bytes); + count_objcg_events(objcg, ZSWPOUT, nr_pages); + } put_pool: zswap_pool_put(pool); put_objcg:
Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") mistakenly skipped charging any zswapped pages when a single call to zswap_store_page() failed, even if some pages in the folio are successfully stored in zswap. Making things worse, these not-charged pages are uncharged in zswap_entry_free(), making zswap charging inconsistent. This inconsistency triggers two warnings when following these 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 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)) Charge zswapped pages even if some pages of the folio are not zswapped. After resolving the inconsistency, these warnings disappear. Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()") Cc: stable@vger.kernel.org Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> --- mm/zswap.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)