The patch titled
Subject: mm: enforce a minimal stack gap even against inaccessible VMAs
has been added to the -mm mm-hotfixes-unstable branch. Its filename is
mm-enforce-a-minimal-stack-gap-even-against-inaccessible-vmas.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patche…
This patch will later appear in the mm-hotfixes-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Jann Horn <jannh(a)google.com>
Subject: mm: enforce a minimal stack gap even against inaccessible VMAs
Date: Tue, 08 Oct 2024 00:55:39 +0200
As explained in the comment block this change adds, we can't tell what
userspace's intent is when the stack grows towards an inaccessible VMA.
I have a (highly contrived) C testcase for 32-bit x86 userspace with glibc
that mixes malloc(), pthread creation, and recursion in just the right way
such that the main stack overflows into malloc() arena memory.
I don't know of any specific scenario where this is actually exploitable,
but it seems like it could be a security problem for sufficiently unlucky
userspace.
I believe we should ensure that, as long as code is compiled with
something like -fstack-check, a stack overflow in it can never cause the
main stack to overflow into adjacent heap memory.
My fix effectively reverts the behavior for !vma_is_accessible() VMAs to
the behavior before commit 1be7107fbe18 ("mm: larger stack guard gap,
between vmas"), so I think it should be a fairly safe change even in case
A.
Link: https://lkml.kernel.org/r/20241008-stack-gap-inaccessible-v1-1-848d4d891f21…
Fixes: 561b5e0709e4 ("mm/mmap.c: do not blow on PROT_NONE MAP_FIXED holes in the stack")
Signed-off-by: Jann Horn <jannh(a)google.com>
Cc: Ben Hutchings <ben(a)decadent.org.uk>
Cc: Helge Deller <deller(a)gmx.de>
Cc: Hugh Dickins <hughd(a)google.com>
Cc: Michal Hocko <mhocko(a)suse.com>
Cc: Oleg Nesterov <oleg(a)redhat.com>
Cc: Rik van Riel <riel(a)redhat.com>
Cc: Vlastimil Babka <vbabka(a)suse.cz>
Cc: Willy Tarreau <w(a)1wt.eu>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
mm/mmap.c | 53 +++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 7 deletions(-)
--- a/mm/mmap.c~mm-enforce-a-minimal-stack-gap-even-against-inaccessible-vmas
+++ a/mm/mmap.c
@@ -1064,10 +1064,12 @@ static int expand_upwards(struct vm_area
gap_addr = TASK_SIZE;
next = find_vma_intersection(mm, vma->vm_end, gap_addr);
- if (next && vma_is_accessible(next)) {
- if (!(next->vm_flags & VM_GROWSUP))
+ if (next && !(next->vm_flags & VM_GROWSUP)) {
+ /* see comments in expand_downwards() */
+ if (vma_is_accessible(prev))
+ return -ENOMEM;
+ if (address == next->vm_start)
return -ENOMEM;
- /* Check that both stack segments have the same anon_vma? */
}
if (next)
@@ -1155,10 +1157,47 @@ int expand_downwards(struct vm_area_stru
/* Enforce stack_guard_gap */
prev = vma_prev(&vmi);
/* Check that both stack segments have the same anon_vma? */
- if (prev) {
- if (!(prev->vm_flags & VM_GROWSDOWN) &&
- vma_is_accessible(prev) &&
- (address - prev->vm_end < stack_guard_gap))
+ if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
+ (address - prev->vm_end < stack_guard_gap)) {
+ /*
+ * If the previous VMA is accessible, this is the normal case
+ * where the main stack is growing down towards some unrelated
+ * VMA. Enforce the full stack guard gap.
+ */
+ if (vma_is_accessible(prev))
+ return -ENOMEM;
+
+ /*
+ * If the previous VMA is not accessible, we have a problem:
+ * We can't tell what userspace's intent is.
+ *
+ * Case A:
+ * Maybe userspace wants to use the previous VMA as a
+ * "guard region" at the bottom of the main stack, in which case
+ * userspace wants us to grow the stack until it is adjacent to
+ * the guard region. Apparently some Java runtime environments
+ * and Rust do that?
+ * That is kind of ugly, and in that case userspace really ought
+ * to ensure that the stack is fully expanded immediately, but
+ * we have to handle this case.
+ *
+ * Case B:
+ * But maybe the previous VMA is entirely unrelated to the stack
+ * and is only *temporarily* PROT_NONE. For example, glibc
+ * malloc arenas create a big PROT_NONE region and then
+ * progressively mark parts of it as writable.
+ * In that case, we must not let the stack become adjacent to
+ * the previous VMA. Otherwise, after the region later becomes
+ * writable, a stack overflow will cause the stack to grow into
+ * the previous VMA, and we won't have any stack gap to protect
+ * against this.
+ *
+ * As an ugly tradeoff, enforce a single-page gap.
+ * A single page will hopefully be small enough to not be
+ * noticed in case A, while providing the same level of
+ * protection in case B that normal userspace threads get.
+ */
+ if (address == prev->vm_end)
return -ENOMEM;
}
_
Patches currently in -mm which might be from jannh(a)google.com are
mm-mremap-prevent-racing-change-of-old-pmd-type.patch
mm-enforce-a-minimal-stack-gap-even-against-inaccessible-vmas.patch
We have recently noticed the exact same KASAN splat as in commit
6cd4a78d962b ("net: do not leave a dangling sk pointer, when socket
creation fails"). The problem is that commit did not fully address the
problem, as some pf->create implementations do not use sk_common_release
in their error paths.
For example, we can use the same reproducer as in the above commit, but
changing ping to arping. arping uses AF_PACKET socket and if packet_create
fails, it will just sk_free the allocated sk object.
While we could chase all the pf->create implementations and make sure they
NULL the freed sk object on error from the socket, we can't guarantee
future protocols will not make the same mistake.
So it is easier to just explicitly NULL the sk pointer upon return from
pf->create in __sock_create. We do know that pf->create always releases the
allocated sk object on error, so if the pointer is not NULL, it is
definitely dangling.
Fixes: 6cd4a78d962b ("net: do not leave a dangling sk pointer, when socket creation fails")
Signed-off-by: Ignat Korchagin <ignat(a)cloudflare.com>
Cc: stable(a)vger.kernel.org
---
net/core/sock.c | 3 ---
net/socket.c | 7 ++++++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index 039be95c40cf..e6e04081949c 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3819,9 +3819,6 @@ void sk_common_release(struct sock *sk)
sk->sk_prot->unhash(sk);
- if (sk->sk_socket)
- sk->sk_socket->sk = NULL;
-
/*
* In this point socket cannot receive new packets, but it is possible
* that some packets are in flight because some CPU runs receiver and
diff --git a/net/socket.c b/net/socket.c
index 601ad74930ef..042451f01c65 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1574,8 +1574,13 @@ int __sock_create(struct net *net, int family, int type, int protocol,
rcu_read_unlock();
err = pf->create(net, sock, protocol, kern);
- if (err < 0)
+ if (err < 0) {
+ /* ->create should release the allocated sock->sk object on error
+ * but it may leave the dangling pointer
+ */
+ sock->sk = NULL;
goto out_module_put;
+ }
/*
* Now to bump the refcnt of the [loadable] module that owns this
--
2.39.5
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.4.y
git checkout FETCH_HEAD
git cherry-pick -x 45bb63ed20e02ae146336412889fe5450316a84f
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024100728-graves-septic-4380@gregkh' --subject-prefix 'PATCH 5.4.y' HEAD^..
Possible dependencies:
45bb63ed20e0 ("nfsd: fix delegation_blocked() to block correctly for at least 30 seconds")
b3f255ef6bff ("nfsd: use ktime_get_seconds() for timestamps")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 45bb63ed20e02ae146336412889fe5450316a84f Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb(a)suse.de>
Date: Mon, 9 Sep 2024 15:06:36 +1000
Subject: [PATCH] nfsd: fix delegation_blocked() to block correctly for at
least 30 seconds
The pair of bloom filtered used by delegation_blocked() was intended to
block delegations on given filehandles for between 30 and 60 seconds. A
new filehandle would be recorded in the "new" bit set. That would then
be switch to the "old" bit set between 0 and 30 seconds later, and it
would remain as the "old" bit set for 30 seconds.
Unfortunately the code intended to clear the old bit set once it reached
30 seconds old, preparing it to be the next new bit set, instead cleared
the *new* bit set before switching it to be the old bit set. This means
that the "old" bit set is always empty and delegations are blocked
between 0 and 30 seconds.
This patch updates bd->new before clearing the set with that index,
instead of afterwards.
Reported-by: Olga Kornievskaia <okorniev(a)redhat.com>
Cc: stable(a)vger.kernel.org
Fixes: 6282cd565553 ("NFSD: Don't hand out delegations for 30 seconds after recalling them.")
Signed-off-by: NeilBrown <neilb(a)suse.de>
Reviewed-by: Benjamin Coddington <bcodding(a)redhat.com>
Reviewed-by: Jeff Layton <jlayton(a)kernel.org>
Signed-off-by: Chuck Lever <chuck.lever(a)oracle.com>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cb5a9ab451c5..ac1859c7cc9d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1078,7 +1078,8 @@ static void nfs4_free_deleg(struct nfs4_stid *stid)
* When a delegation is recalled, the filehandle is stored in the "new"
* filter.
* Every 30 seconds we swap the filters and clear the "new" one,
- * unless both are empty of course.
+ * unless both are empty of course. This results in delegations for a
+ * given filehandle being blocked for between 30 and 60 seconds.
*
* Each filter is 256 bits. We hash the filehandle to 32bit and use the
* low 3 bytes as hash-table indices.
@@ -1107,9 +1108,9 @@ static int delegation_blocked(struct knfsd_fh *fh)
if (ktime_get_seconds() - bd->swap_time > 30) {
bd->entries -= bd->old_entries;
bd->old_entries = bd->entries;
+ bd->new = 1-bd->new;
memset(bd->set[bd->new], 0,
sizeof(bd->set[0]));
- bd->new = 1-bd->new;
bd->swap_time = ktime_get_seconds();
}
spin_unlock(&blocked_delegations_lock);
commit 45bb63ed20e02ae146336412889fe5450316a84f
The pair of bloom filtered used by delegation_blocked() was intended to
block delegations on given filehandles for between 30 and 60 seconds. A
new filehandle would be recorded in the "new" bit set. That would then
be switch to the "old" bit set between 0 and 30 seconds later, and it
would remain as the "old" bit set for 30 seconds.
Unfortunately the code intended to clear the old bit set once it reached
30 seconds old, preparing it to be the next new bit set, instead cleared
the *new* bit set before switching it to be the old bit set. This means
that the "old" bit set is always empty and delegations are blocked
between 0 and 30 seconds.
This patch updates bd->new before clearing the set with that index,
instead of afterwards.
Reported-by: Olga Kornievskaia <okorniev(a)redhat.com>
Cc: stable(a)vger.kernel.org
Fixes: 6282cd565553 ("NFSD: Don't hand out delegations for 30 seconds after recalling them.")
Signed-off-by: NeilBrown <neilb(a)suse.de>
Reviewed-by: Benjamin Coddington <bcodding(a)redhat.com>
Reviewed-by: Jeff Layton <jlayton(a)kernel.org>
Signed-off-by: Chuck Lever <chuck.lever(a)oracle.com>
---
fs/nfsd/nfs4state.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7ac644d64ab1..d45487d82d44 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -743,7 +743,8 @@ static void nfs4_free_deleg(struct nfs4_stid *stid)
* When a delegation is recalled, the filehandle is stored in the "new"
* filter.
* Every 30 seconds we swap the filters and clear the "new" one,
- * unless both are empty of course.
+ * unless both are empty of course. This results in delegations for a
+ * given filehandle being blocked for between 30 and 60 seconds.
*
* Each filter is 256 bits. We hash the filehandle to 32bit and use the
* low 3 bytes as hash-table indices.
@@ -772,9 +773,9 @@ static int delegation_blocked(struct knfsd_fh *fh)
if (seconds_since_boot() - bd->swap_time > 30) {
bd->entries -= bd->old_entries;
bd->old_entries = bd->entries;
+ bd->new = 1-bd->new;
memset(bd->set[bd->new], 0,
sizeof(bd->set[0]));
- bd->new = 1-bd->new;
bd->swap_time = seconds_since_boot();
}
spin_unlock(&blocked_delegations_lock);
base-commit: de2cffe297563c815c840cfa14b77a0868b61e53
--
2.46.0
We have recently noticed the exact same KASAN splat as in commit
6cd4a78d962b ("net: do not leave a dangling sk pointer, when socket
creation fails"). The problem is that commit did not fully address the
problem, as some pf->create implementations do not use sk_common_release
in their error paths.
For example, we can use the same reproducer as in the above commit, but
changing ping to arping. arping uses AF_PACKET socket and if packet_create
fails, it will just sk_free the allocated sk object.
While we could chase all the pf->create implementations and make sure they
NULL the freed sk object on error from the socket, we can't guarantee
future protocols will not make the same mistake.
So it is easier to just explicitly NULL the sk pointer upon return from
pf->create in __sock_create. We do know that pf->create always releases the
allocated sk object on error, so if the pointer is not NULL, it is
definitely dangling.
Fixes: 6cd4a78d962b ("net: do not leave a dangling sk pointer, when socket creation fails")
Signed-off-by: Ignat Korchagin <ignat(a)cloudflare.com>
Cc: stable(a)vger.kernel.org
---
net/socket.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/socket.c b/net/socket.c
index 7b046dd3e9a7..19afac3c2de9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1575,8 +1575,13 @@ int __sock_create(struct net *net, int family, int type, int protocol,
rcu_read_unlock();
err = pf->create(net, sock, protocol, kern);
- if (err < 0)
+ if (err < 0) {
+ /* ->create should release the allocated sock->sk object on error
+ * but it may leave the dangling pointer
+ */
+ sock->sk = NULL;
goto out_module_put;
+ }
/*
* Now to bump the refcnt of the [loadable] module that owns this
--
2.39.5
Hello,
I hope this email finds you well. I reached out to you a few hours ago, but I have not received a response yet. I just wanted to check in to see if you had the chance to review my previous message or if there were any issues that may have prevented you from getting back to me.
Could you please confirm receipt of my earlier correspondence and provide a response at your earliest convenience? Your prompt attention to this matter is greatly appreciated.
Thank you for your cooperation.
Warm regards,
Dr. Allen Cheng
Human Resource Manager | Product Research Assistant
FC Industrial Laboratories Ltd
This electronic mail and its attachment(s) is intended only for the recipient(s) to whom it is addressed. It may contain information which may be confidential and/or protected by legal privilege. If you are not the intended recipient(s), reading, disclosing, printing, copying, forwarding this electronic mail and its attachment(s) and/or taking any action in reliance on the information in this electronic mail and its attachment(s) are prohibited. Koperasi Telkomsel/Koperasi Telekomunikasi Selular/Kisel shall not be liable in respect of communication made by its employee which is contrary to the company policy and/or outside the scope of the employment of the individual concerned. The employee will be personally liable for any damages or other liability arising
Surat elektronik ini beserta lampirannya dimaksudkan hanya untuk penerima kepada siapa surat tersebut ditujukan. Informasi yang terdapat di dalamnya dapat bersifat rahasia dan/atau dilindungi oleh hukum. Jika Anda bukan penerima yang dituju, Anda dilarang untuk membaca, mengungkapkan, mencetak, menduplikasi/menyalin, meneruskan surat elektronik ini beserta lampirannya dan/atau mengambil tindakan apapun berdasarkan informasi yang terdapat dalam surat elektronik ini beserta lampirannya. Koperasi Telkomsel/Koperasi Telekomunikasi Selular/Kisel tidak bertanggung jawab atas setiap komunikasi karyawan yang bertentangan dengan kebijakan perusahaan dan/atau berada di luar lingkup pekerjaannya. Segala resiko dan akibat yang ditimbulkan merupakan tanggung jawab personal masing-masing.
Commit 9bf4e919ccad worked around an issue introduced after an innocuous
optimisation change in LLVM main:
> len is defined as an 'int' because it is assigned from
> '__user int *optlen'. However, it is clamped against the result of
> sizeof(), which has a type of 'size_t' ('unsigned long' for 64-bit
> platforms). This is done with min_t() because min() requires compatible
> types, which results in both len and the result of sizeof() being casted
> to 'unsigned int', meaning len changes signs and the result of sizeof()
> is truncated. From there, len is passed to copy_to_user(), which has a
> third parameter type of 'unsigned long', so it is widened and changes
> signs again. This excessive casting in combination with the KCSAN
> instrumentation causes LLVM to fail to eliminate the __bad_copy_from()
> call, failing the build.
The same issue occurs in rfcomm in functions rfcomm_sock_bind and
rfcomm_sock_getsockopt_old.
Change the type of len to size_t in both rfcomm_sock_bind and
rfcomm_sock_getsockopt_old and replace min_t() with min().
Cc: stable(a)vger.kernel.org
Fixes: 9bf4e919ccad ("Bluetooth: Fix type of len in {l2cap,sco}_sock_getsockopt_old()")
Link: https://github.com/ClangBuiltLinux/linux/issues/2007
Link: https://github.com/llvm/llvm-project/issues/85647
Signed-off-by: Andrej Shadura <andrew.shadura(a)collabora.co.uk>
---
net/bluetooth/rfcomm/sock.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 37d63d768afb..c0fe96673b3c 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -328,14 +328,15 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
{
struct sockaddr_rc sa;
struct sock *sk = sock->sk;
- int len, err = 0;
+ int err = 0;
+ size_t len;
if (!addr || addr_len < offsetofend(struct sockaddr, sa_family) ||
addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
memset(&sa, 0, sizeof(sa));
- len = min_t(unsigned int, sizeof(sa), addr_len);
+ len = min(sizeof(sa), addr_len);
memcpy(&sa, addr, len);
BT_DBG("sk %p %pMR", sk, &sa.rc_bdaddr);
@@ -729,7 +730,8 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u
struct sock *l2cap_sk;
struct l2cap_conn *conn;
struct rfcomm_conninfo cinfo;
- int len, err = 0;
+ int err = 0;
+ size_t len;
u32 opt;
BT_DBG("sk %p", sk);
@@ -783,7 +785,7 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u
cinfo.hci_handle = conn->hcon->handle;
memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);
- len = min_t(unsigned int, len, sizeof(cinfo));
+ len = min(len, sizeof(cinfo));
if (copy_to_user(optval, (char *) &cinfo, len))
err = -EFAULT;
--
2.43.0
The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 7a2369b74abf76cd3e54c45b30f6addb497f831b
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024100711-ebook-refund-46f3@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
7a2369b74abf ("mm: z3fold: deprecate CONFIG_Z3FOLD")
04cb7502a5d7 ("zsmalloc: use all available 24 bits of page_type")
43d746dc49bb ("mm/zsmalloc: use a proper page type")
8db00ad56461 ("mm: allow reuse of the lower 16 bit of the page type with an actual type")
6d21dde7adc0 ("mm: update _mapcount and page_type documentation")
ff202303c398 ("mm: convert page type macros to enum")
46df8e73a4a3 ("mm: free up PG_slab")
d99e3140a4d3 ("mm: turn folio_test_hugetlb into a PageType")
fd1a745ce03e ("mm: support page_mapcount() on page_has_type() pages")
29cfe7556bfd ("mm: constify more page/folio tests")
443cbaf9e2fd ("crash: split vmcoreinfo exporting code out from crash_core.c")
85fcde402db1 ("kexec: split crashkernel reservation code out from crash_core.c")
55c49fee57af ("mm/vmalloc: remove vmap_area_list")
d093602919ad ("mm: vmalloc: remove global vmap_area_root rb-tree")
7fa8cee00316 ("mm: vmalloc: move vmap_init_free_space() down in vmalloc.c")
4a693ce65b18 ("kdump: defer the insertion of crashkernel resources")
9f2a63523582 ("Merge tag 'mm-nonmm-stable-2024-01-09-10-33' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 7a2369b74abf76cd3e54c45b30f6addb497f831b Mon Sep 17 00:00:00 2001
From: Yosry Ahmed <yosryahmed(a)google.com>
Date: Wed, 4 Sep 2024 23:33:43 +0000
Subject: [PATCH] mm: z3fold: deprecate CONFIG_Z3FOLD
The z3fold compressed pages allocator is rarely used, most users use
zsmalloc. The only disadvantage of zsmalloc in comparison is the
dependency on MMU, and zbud is a more common option for !MMU as it was the
default zswap allocator for a long time.
Historically, zsmalloc had worse latency than zbud and z3fold but offered
better memory savings. This is no longer the case as shown by a simple
recent analysis [1]. That analysis showed that z3fold does not have any
advantage over zsmalloc or zbud considering both performance and memory
usage. In a kernel build test on tmpfs in a limited cgroup, z3fold took
3% more time and used 1.8% more memory. The latency of zswap_load() was
7% higher, and that of zswap_store() was 10% higher. Zsmalloc is better
in all metrics.
Moreover, z3fold apparently has latent bugs, which was made noticeable by
a recent soft lockup bug report with z3fold [2]. Switching to zsmalloc
not only fixed the problem, but also reduced the swap usage from 6~8G to
1~2G. Other users have also reported being bitten by mistakenly enabling
z3fold.
Other than hurting users, z3fold is repeatedly causing wasted engineering
effort. Apart from investigating the above bug, it came up in multiple
development discussions (e.g. [3]) as something we need to handle, when
there aren't any legit users (at least not intentionally).
The natural course of action is to deprecate z3fold, and remove in a few
cycles if no objections are raised from active users. Next on the list
should be zbud, as it offers marginal latency gains at the cost of huge
memory waste when compared to zsmalloc. That one will need to wait until
zsmalloc does not depend on MMU.
Rename the user-visible config option from CONFIG_Z3FOLD to
CONFIG_Z3FOLD_DEPRECATED so that users with CONFIG_Z3FOLD=y get a new
prompt with explanation during make oldconfig. Also, remove
CONFIG_Z3FOLD=y from defconfigs.
[1]https://lore.kernel.org/lkml/CAJD7tkbRF6od-2x_L8-A1QL3=2Ww13sCj4S3i4bNndq…
[2]https://lore.kernel.org/lkml/EF0ABD3E-A239-4111-A8AB-5C442E759CF3@gmail.c…
[3]https://lore.kernel.org/lkml/CAJD7tkbnmeVugfunffSovJf9FAgy9rhBVt_tx=nxUve…
[arnd(a)arndb.de: deprecate ZSWAP_ZPOOL_DEFAULT_Z3FOLD as well]
Link: https://lkml.kernel.org/r/20240909202625.1054880-1-arnd@kernel.org
Link: https://lkml.kernel.org/r/20240904233343.933462-1-yosryahmed@google.com
Signed-off-by: Yosry Ahmed <yosryahmed(a)google.com>
Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
Acked-by: Chris Down <chris(a)chrisdown.name>
Acked-by: Nhat Pham <nphamcs(a)gmail.com>
Acked-by: Johannes Weiner <hannes(a)cmpxchg.org>
Acked-by: Vitaly Wool <vitaly.wool(a)konsulko.com>
Acked-by: Christoph Hellwig <hch(a)lst.de>
Cc: Aneesh Kumar K.V <aneesh.kumar(a)kernel.org>
Cc: Christophe Leroy <christophe.leroy(a)csgroup.eu>
Cc: Huacai Chen <chenhuacai(a)kernel.org>
Cc: Miaohe Lin <linmiaohe(a)huawei.com>
Cc: Michael Ellerman <mpe(a)ellerman.id.au>
Cc: Naveen N. Rao <naveen.n.rao(a)linux.ibm.com>
Cc: Nicholas Piggin <npiggin(a)gmail.com>
Cc: Sergey Senozhatsky <senozhatsky(a)chromium.org>
Cc: WANG Xuerui <kernel(a)xen0n.name>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index b4252c357c8e..75b366407a60 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -96,7 +96,6 @@ CONFIG_ZPOOL=y
CONFIG_ZSWAP=y
CONFIG_ZSWAP_COMPRESSOR_DEFAULT_ZSTD=y
CONFIG_ZBUD=y
-CONFIG_Z3FOLD=y
CONFIG_ZSMALLOC=m
# CONFIG_COMPAT_BRK is not set
CONFIG_MEMORY_HOTPLUG=y
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 544a65fda77b..d39284489aa2 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -81,7 +81,6 @@ CONFIG_MODULE_SIG_SHA512=y
CONFIG_PARTITION_ADVANCED=y
CONFIG_BINFMT_MISC=m
CONFIG_ZSWAP=y
-CONFIG_Z3FOLD=y
CONFIG_ZSMALLOC=y
# CONFIG_SLAB_MERGE_DEFAULT is not set
CONFIG_SLAB_FREELIST_RANDOM=y
diff --git a/mm/Kconfig b/mm/Kconfig
index 1aa282e35dc7..09aebca1cae3 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -146,12 +146,15 @@ config ZSWAP_ZPOOL_DEFAULT_ZBUD
help
Use the zbud allocator as the default allocator.
-config ZSWAP_ZPOOL_DEFAULT_Z3FOLD
- bool "z3fold"
- select Z3FOLD
+config ZSWAP_ZPOOL_DEFAULT_Z3FOLD_DEPRECATED
+ bool "z3foldi (DEPRECATED)"
+ select Z3FOLD_DEPRECATED
help
Use the z3fold allocator as the default allocator.
+ Deprecated and scheduled for removal in a few cycles,
+ see CONFIG_Z3FOLD_DEPRECATED.
+
config ZSWAP_ZPOOL_DEFAULT_ZSMALLOC
bool "zsmalloc"
select ZSMALLOC
@@ -163,7 +166,7 @@ config ZSWAP_ZPOOL_DEFAULT
string
depends on ZSWAP
default "zbud" if ZSWAP_ZPOOL_DEFAULT_ZBUD
- default "z3fold" if ZSWAP_ZPOOL_DEFAULT_Z3FOLD
+ default "z3fold" if ZSWAP_ZPOOL_DEFAULT_Z3FOLD_DEPRECATED
default "zsmalloc" if ZSWAP_ZPOOL_DEFAULT_ZSMALLOC
default ""
@@ -177,15 +180,25 @@ config ZBUD
deterministic reclaim properties that make it preferable to a higher
density approach when reclaim will be used.
-config Z3FOLD
- tristate "3:1 compression allocator (z3fold)"
+config Z3FOLD_DEPRECATED
+ tristate "3:1 compression allocator (z3fold) (DEPRECATED)"
depends on ZSWAP
help
+ Deprecated and scheduled for removal in a few cycles. If you have
+ a good reason for using Z3FOLD over ZSMALLOC, please contact
+ linux-mm(a)kvack.org and the zswap maintainers.
+
A special purpose allocator for storing compressed pages.
It is designed to store up to three compressed pages per physical
page. It is a ZBUD derivative so the simplicity and determinism are
still there.
+config Z3FOLD
+ tristate
+ default y if Z3FOLD_DEPRECATED=y
+ default m if Z3FOLD_DEPRECATED=m
+ depends on Z3FOLD_DEPRECATED
+
config ZSMALLOC
tristate
prompt "N:1 compression allocator (zsmalloc)" if (ZSWAP || ZRAM)