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);
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:
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: The upstream commit ID must be specified with a separate line above the commit text. Subject: [PATCH v6.12 hotfix] mm/zswap: fix inconsistent charging when zswap_store_page() fails Link: https://lore.kernel.org/stable/20250128174938.2638-1-42.hyeyoo%40gmail.com
Please ignore this mail if the patch is not relevant for upstream.
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 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 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 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
[ Sasha's backport helper bot ]
Hi,
No upstream commit was identified. Using temporary commit for testing.
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.12.y | Failed | N/A |
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
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Hyeonggon Yoo -
kernel test robot -
Sasha Levin -
Vlastimil Babka