Linux userland API discussions
 help / color / mirror / Atom feed
* [RFC -next 04/10] splice: Add SPLICE_F_ZC and attach ubuf
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

Add the SPLICE_F_ZC flag and when it is set, allocate a ubuf and attach
it to generate zerocopy notifications.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 fs/splice.c            | 20 ++++++++++++++++++++
 include/linux/splice.h |  3 ++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/splice.c b/fs/splice.c
index 1f27ce6d1c34..6dc60f47f84e 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -875,6 +875,11 @@ static ssize_t splice_socket_generic(struct pipe_inode_info *pipe,
 		if (out->f_flags & O_NONBLOCK)
 			msg.msg_flags |= MSG_DONTWAIT;
 
+		if (unlikely(flags & SPLICE_F_ZC) && ubuf_info) {
+			msg.msg_flags = MSG_ZEROCOPY;
+			msg.msg_ubuf = ubuf_info;
+		}
+
 		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, bvec, bc,
 			      len - remain);
 		ret = sock_sendmsg(sock, &msg);
@@ -1223,12 +1228,27 @@ static ssize_t do_splice_direct_actor(struct file *in, loff_t *ppos,
 	if (unlikely(out->f_flags & O_APPEND))
 		return -EINVAL;
 
+	if (unlikely(flags & SPLICE_F_ZC)) {
+		struct socket *sock = sock_from_file(out);
+		struct sock *sk = sock->sk;
+		struct ubuf_info *ubuf_info;
+
+		ubuf_info = msg_zerocopy_realloc(sk, len, NULL);
+		if (!ubuf_info)
+			return -ENOMEM;
+		sd.ubuf_info = ubuf_info;
+	}
+
 	ret = splice_direct_to_actor(in, &sd, actor);
 	if (ret > 0)
 		*ppos = sd.pos;
 
+	if (unlikely(flags & SPLICE_F_ZC))
+		refcount_dec(&sd.ubuf_info->refcnt);
+
 	return ret;
 }
+
 /**
  * do_splice_direct - splices data directly between two files
  * @in:		file to splice from
diff --git a/include/linux/splice.h b/include/linux/splice.h
index 7477df3916e2..a88588cf2754 100644
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -21,8 +21,9 @@
 				 /* from/to, of course */
 #define SPLICE_F_MORE	(0x04)	/* expect more data */
 #define SPLICE_F_GIFT	(0x08)	/* pages passed in are a gift */
+#define SPLICE_F_ZC	(0x10)  /* generate zero copy notifications */
 
-#define SPLICE_F_ALL (SPLICE_F_MOVE|SPLICE_F_NONBLOCK|SPLICE_F_MORE|SPLICE_F_GIFT)
+#define SPLICE_F_ALL (SPLICE_F_MOVE|SPLICE_F_NONBLOCK|SPLICE_F_MORE|SPLICE_F_GIFT|SPLICE_F_ZC)
 
 /*
  * Passed to the actors
-- 
2.43.0


^ permalink raw reply related

* [RFC -next 05/10] fs: Add splice_write_sd to file operations
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

Introduce splice_write_sd to file operations and export a new helper for
sockets splice_to_socket_sd to pass through the splice_desc context
allowing the allocated ubuf to be attached.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 fs/splice.c            | 22 ++++++++++++++++++----
 include/linux/fs.h     |  2 ++
 include/linux/splice.h |  2 ++
 net/socket.c           |  1 +
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/fs/splice.c b/fs/splice.c
index 6dc60f47f84e..d08fa2a6d930 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -935,6 +935,16 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 {
 	return splice_socket_generic(pipe, out, ppos, len, flags, NULL);
 }
+
+ssize_t splice_to_socket_sd(struct pipe_inode_info *pipe,
+			    struct file *out, struct splice_desc *sd)
+{
+	ssize_t ret;
+
+	ret = splice_socket_generic(pipe, out, sd->opos, sd->total_len,
+				    sd->flags, sd->ubuf_info);
+	return ret;
+}
 #endif
 
 static int warn_unsupported(struct file *file, const char *op)
@@ -959,10 +969,14 @@ static ssize_t do_splice_from(struct pipe_inode_info *pipe, struct file *out,
 static ssize_t do_splice_from_sd(struct pipe_inode_info *pipe, struct file *out,
 				 struct splice_desc *sd)
 {
-	if (unlikely(!out->f_op->splice_write))
-		return warn_unsupported(out, "write");
-	return out->f_op->splice_write(pipe, out, sd->opos, sd->total_len,
-				       sd->flags);
+	if (likely(!(sd->flags & SPLICE_F_ZC))) {
+		if (unlikely(!out->f_op->splice_write))
+			return warn_unsupported(out, "write");
+		return out->f_op->splice_write(pipe, out, sd->opos,
+					       sd->total_len, sd->flags);
+	} else {
+		return out->f_op->splice_write_sd(pipe, out, sd);
+	}
 }
 
 /*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7e29433c5ecc..843e8b8a1d4d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2065,6 +2065,7 @@ struct dir_context {
 struct iov_iter;
 struct io_uring_cmd;
 struct offset_ctx;
+struct splice_desc;
 
 typedef unsigned int __bitwise fop_flags_t;
 
@@ -2093,6 +2094,7 @@ struct file_operations {
 	int (*check_flags)(int);
 	int (*flock) (struct file *, int, struct file_lock *);
 	ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int);
+	ssize_t (*splice_write_sd)(struct pipe_inode_info *, struct file *, struct splice_desc *);
 	ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int);
 	void (*splice_eof)(struct file *file);
 	int (*setlease)(struct file *, int, struct file_lease **, void **);
diff --git a/include/linux/splice.h b/include/linux/splice.h
index a88588cf2754..356b8cae4818 100644
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -100,6 +100,8 @@ static inline long splice_copy_file_range(struct file *in, loff_t pos_in,
 
 ssize_t do_tee(struct file *in, struct file *out, size_t len,
 	       unsigned int flags);
+ssize_t splice_to_socket_sd(struct pipe_inode_info *pipe, struct file *out,
+			    struct splice_desc *sd);
 ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 			 loff_t *ppos, size_t len, unsigned int flags);
 
diff --git a/net/socket.c b/net/socket.c
index 9a117248f18f..4baf26a36477 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -165,6 +165,7 @@ static const struct file_operations socket_file_ops = {
 	.release =	sock_close,
 	.fasync =	sock_fasync,
 	.splice_write = splice_to_socket,
+	.splice_write_sd = splice_to_socket_sd,
 	.splice_read =	sock_splice_read,
 	.splice_eof =	sock_splice_eof,
 	.show_fdinfo =	sock_show_fdinfo,
-- 
2.43.0


^ permalink raw reply related

* [RFC -next 06/10] fs: Extend do_sendfile to take a flags argument
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

Extend the internal do_sendfile to take a flags argument, which will be
used in future commits to signal that userland wants zerocopy
notifications.

This commit does not change anything about sendfile or sendfile64.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 fs/read_write.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index a6133241dfb8..03d2a93c3d1b 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1293,7 +1293,7 @@ COMPAT_SYSCALL_DEFINE6(pwritev2, compat_ulong_t, fd,
 #endif /* CONFIG_COMPAT */
 
 static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
-			   size_t count, loff_t max)
+			   size_t count, loff_t max, int flags)
 {
 	struct inode *in_inode, *out_inode;
 	struct pipe_inode_info *opipe;
@@ -1398,13 +1398,13 @@ SYSCALL_DEFINE4(sendfile, int, out_fd, int, in_fd, off_t __user *, offset, size_
 		if (unlikely(get_user(off, offset)))
 			return -EFAULT;
 		pos = off;
-		ret = do_sendfile(out_fd, in_fd, &pos, count, MAX_NON_LFS);
+		ret = do_sendfile(out_fd, in_fd, &pos, count, MAX_NON_LFS, 0);
 		if (unlikely(put_user(pos, offset)))
 			return -EFAULT;
 		return ret;
 	}
 
-	return do_sendfile(out_fd, in_fd, NULL, count, 0);
+	return do_sendfile(out_fd, in_fd, NULL, count, 0, 0);
 }
 
 SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd, loff_t __user *, offset, size_t, count)
@@ -1415,13 +1415,13 @@ SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd, loff_t __user *, offset, si
 	if (offset) {
 		if (unlikely(copy_from_user(&pos, offset, sizeof(loff_t))))
 			return -EFAULT;
-		ret = do_sendfile(out_fd, in_fd, &pos, count, 0);
+		ret = do_sendfile(out_fd, in_fd, &pos, count, 0, 0);
 		if (unlikely(put_user(pos, offset)))
 			return -EFAULT;
 		return ret;
 	}
 
-	return do_sendfile(out_fd, in_fd, NULL, count, 0);
+	return do_sendfile(out_fd, in_fd, NULL, count, 0, 0);
 }
 
 #ifdef CONFIG_COMPAT
@@ -1436,13 +1436,13 @@ COMPAT_SYSCALL_DEFINE4(sendfile, int, out_fd, int, in_fd,
 		if (unlikely(get_user(off, offset)))
 			return -EFAULT;
 		pos = off;
-		ret = do_sendfile(out_fd, in_fd, &pos, count, MAX_NON_LFS);
+		ret = do_sendfile(out_fd, in_fd, &pos, count, MAX_NON_LFS, 0);
 		if (unlikely(put_user(pos, offset)))
 			return -EFAULT;
 		return ret;
 	}
 
-	return do_sendfile(out_fd, in_fd, NULL, count, 0);
+	return do_sendfile(out_fd, in_fd, NULL, count, 0, 0);
 }
 
 COMPAT_SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd,
@@ -1454,13 +1454,13 @@ COMPAT_SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd,
 	if (offset) {
 		if (unlikely(copy_from_user(&pos, offset, sizeof(loff_t))))
 			return -EFAULT;
-		ret = do_sendfile(out_fd, in_fd, &pos, count, 0);
+		ret = do_sendfile(out_fd, in_fd, &pos, count, 0, 0);
 		if (unlikely(put_user(pos, offset)))
 			return -EFAULT;
 		return ret;
 	}
 
-	return do_sendfile(out_fd, in_fd, NULL, count, 0);
+	return do_sendfile(out_fd, in_fd, NULL, count, 0, 0);
 }
 #endif
 
-- 
2.43.0


^ permalink raw reply related

* [RFC -next 07/10] fs: Add sendfile2 which accepts a flags argument
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

Add sendfile2 which is similar to sendfile64, but takes a flags
argument.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 fs/read_write.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/read_write.c b/fs/read_write.c
index 03d2a93c3d1b..057e5f37645d 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1424,6 +1424,23 @@ SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd, loff_t __user *, offset, si
 	return do_sendfile(out_fd, in_fd, NULL, count, 0, 0);
 }
 
+SYSCALL_DEFINE5(sendfile2, int, out_fd, int, in_fd, loff_t __user *, offset, size_t, count, int, flags)
+{
+	loff_t pos;
+	ssize_t ret;
+
+	if (offset) {
+		if (unlikely(copy_from_user(&pos, offset, sizeof(loff_t))))
+			return -EFAULT;
+		ret = do_sendfile(out_fd, in_fd, &pos, count, 0, flags);
+		if (unlikely(put_user(pos, offset)))
+			return -EFAULT;
+		return ret;
+	}
+
+	return do_sendfile(out_fd, in_fd, NULL, count, 0, flags);
+}
+
 #ifdef CONFIG_COMPAT
 COMPAT_SYSCALL_DEFINE4(sendfile, int, out_fd, int, in_fd,
 		compat_off_t __user *, offset, compat_size_t, count)
-- 
2.43.0


^ permalink raw reply related

* [RFC -next 08/10] fs: Add sendfile flags for sendfile2
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

Add a default flag (SENDFILE_DEFAULT) and a flag for requesting zerocopy
notifications (SENDFILE_ZC). do_sendfile is updated to pass through the
corresponding splice flag to enable zerocopy notifications.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 fs/read_write.c          |  5 +++++
 include/linux/sendfile.h | 10 ++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 include/linux/sendfile.h

diff --git a/fs/read_write.c b/fs/read_write.c
index 057e5f37645d..e3929fd0f605 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -16,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/syscalls.h>
 #include <linux/pagemap.h>
+#include <linux/sendfile.h>
 #include <linux/splice.h>
 #include <linux/compat.h>
 #include <linux/mount.h>
@@ -1360,6 +1361,10 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
 		retval = rw_verify_area(WRITE, fd_file(out), &out_pos, count);
 		if (retval < 0)
 			return retval;
+
+		if (flags & SENDFILE_ZC)
+			fl |= SPLICE_F_ZC;
+
 		retval = do_splice_direct(fd_file(in), &pos, fd_file(out), &out_pos,
 					  count, fl);
 	} else {
diff --git a/include/linux/sendfile.h b/include/linux/sendfile.h
new file mode 100644
index 000000000000..0bd3c76ea6f2
--- /dev/null
+++ b/include/linux/sendfile.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef SENDFILE_H
+#define SENDFILE_H
+
+#define SENDFILE_DEFAULT (0x1)  /* normal sendfile */
+#define SENDFILE_ZC (0x2)       /* sendfile which generates ZC notifications */
+
+#define SENDFILE_ALL (SENDFILE_DEFAULT|SENDFILE_ZC)
+
+#endif
-- 
2.43.0


^ permalink raw reply related

* [RFC -next 09/10] fs: Add sendfile2 syscall
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

The sendfile2 system call is similar to sendfile64, but takes a flags
argument allowing the user to select either a default sendfile or for
sendfile to generate zerocopy notifications similar to MSG_ZEROCOPY and
sendmsg.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 arch/alpha/kernel/syscalls/syscall.tbl      | 1 +
 arch/arm/tools/syscall.tbl                  | 1 +
 arch/arm64/tools/syscall_32.tbl             | 1 +
 arch/m68k/kernel/syscalls/syscall.tbl       | 1 +
 arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   | 1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   | 1 +
 arch/mips/kernel/syscalls/syscall_o32.tbl   | 1 +
 arch/parisc/kernel/syscalls/syscall.tbl     | 1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    | 1 +
 arch/s390/kernel/syscalls/syscall.tbl       | 1 +
 arch/sh/kernel/syscalls/syscall.tbl         | 1 +
 arch/sparc/kernel/syscalls/syscall.tbl      | 1 +
 arch/x86/entry/syscalls/syscall_32.tbl      | 1 +
 arch/x86/entry/syscalls/syscall_64.tbl      | 1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     | 1 +
 include/linux/syscalls.h                    | 2 ++
 include/uapi/asm-generic/unistd.h           | 4 +++-
 scripts/syscall.tbl                         | 1 +
 19 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index c59d53d6d3f3..124313c745b6 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -506,3 +506,4 @@
 574	common	getxattrat			sys_getxattrat
 575	common	listxattrat			sys_listxattrat
 576	common	removexattrat			sys_removexattrat
+577	common	sendfile2			sys_sendfile2
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 49eeb2ad8dbd..ca61b5792148 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -481,3 +481,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/arm64/tools/syscall_32.tbl b/arch/arm64/tools/syscall_32.tbl
index 69a829912a05..71695a61a1df 100644
--- a/arch/arm64/tools/syscall_32.tbl
+++ b/arch/arm64/tools/syscall_32.tbl
@@ -478,3 +478,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index f5ed71f1910d..6096a22b4472 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -466,3 +466,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 680f568b77f2..0429dc26ceee 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -472,3 +472,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 0b9b7e25b69a..f6571c8ecb15 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -405,3 +405,4 @@
 464	n32	getxattrat			sys_getxattrat
 465	n32	listxattrat			sys_listxattrat
 466	n32	removexattrat			sys_removexattrat
+467	n32	sendfile2			sys_sendfile2
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index c844cd5cda62..532ce99478ee 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -381,3 +381,4 @@
 464	n64	getxattrat			sys_getxattrat
 465	n64	listxattrat			sys_listxattrat
 466	n64	removexattrat			sys_removexattrat
+467	n64	sendfile2			sys_sendfile2
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 349b8aad1159..9cacbbff6b12 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -454,3 +454,4 @@
 464	o32	getxattrat			sys_getxattrat
 465	o32	listxattrat			sys_listxattrat
 466	o32	removexattrat			sys_removexattrat
+467	o32	sendfile2			sys_sendfile2
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index d9fc94c86965..ca5a3e6eb8f3 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -465,3 +465,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index d8b4ab78bef0..450392aed1eb 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -557,3 +557,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index e9115b4d8b63..e7e1b16f4d39 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -469,3 +469,4 @@
 464  common	getxattrat		sys_getxattrat			sys_getxattrat
 465  common	listxattrat		sys_listxattrat			sys_listxattrat
 466  common	removexattrat		sys_removexattrat		sys_removexattrat
+467  64	sendfile2		sys_sendfile2			-
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c8cad33bf250..c75a0e69c033 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -470,3 +470,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 727f99d333b3..fd15465b5330 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -512,3 +512,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 4d0fb2fba7e2..f711ee6068ec 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -472,3 +472,4 @@
 464	i386	getxattrat		sys_getxattrat
 465	i386	listxattrat		sys_listxattrat
 466	i386	removexattrat		sys_removexattrat
+467	i386    sendfile2		sys_sendfile2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 5eb708bff1c7..0ba4edb1e4c0 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -390,6 +390,7 @@
 464	common	getxattrat		sys_getxattrat
 465	common	listxattrat		sys_listxattrat
 466	common	removexattrat		sys_removexattrat
+467	common  sendfile2		sys_sendfile2
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 37effc1b134e..142597c92baf 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -437,3 +437,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common	sendfile2			sys_sendfile2
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index c6333204d451..3ee0e997d6c6 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -491,6 +491,8 @@ asmlinkage long sys_pwritev(unsigned long fd, const struct iovec __user *vec,
 			    unsigned long vlen, unsigned long pos_l, unsigned long pos_h);
 asmlinkage long sys_sendfile64(int out_fd, int in_fd,
 			       loff_t __user *offset, size_t count);
+asmlinkage long sys_sendfile2(int out_fd, int in_fd,
+			      loff_t __user *offset, size_t count, int flags);
 asmlinkage long sys_pselect6(int, fd_set __user *, fd_set __user *,
 			     fd_set __user *, struct __kernel_timespec __user *,
 			     void __user *);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 88dc393c2bca..ec0ac5a8d519 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -849,9 +849,11 @@ __SYSCALL(__NR_getxattrat, sys_getxattrat)
 __SYSCALL(__NR_listxattrat, sys_listxattrat)
 #define __NR_removexattrat 466
 __SYSCALL(__NR_removexattrat, sys_removexattrat)
+#define __NR_sendfile2 467
+__SYSCALL(__NR_sendfile2, sys_sendfile2)
 
 #undef __NR_syscalls
-#define __NR_syscalls 467
+#define __NR_syscalls 468
 
 /*
  * 32 bit systems traditionally used different
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index ebbdb3c42e9f..1911a64d3b33 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -407,3 +407,4 @@
 464	common	getxattrat			sys_getxattrat
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
+467	common  sendfile2			sys_sendfile2
-- 
2.43.0


^ permalink raw reply related

* [RFC -next 10/10] selftests: Add sendfile zerocopy notification test
From: Joe Damato @ 2025-03-19  0:15 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, asml.silence, linux-fsdevel, edumazet, pabeni,
	horms, linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo,
	arnd, brauner, akpm, tglx, jolsa, linux-kselftest, Joe Damato
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

Extend the existing the msg_zerocopy test to allow testing sendfile to
ensure that notifications are generated.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 tools/testing/selftests/net/msg_zerocopy.c  | 54 ++++++++++++++++++++-
 tools/testing/selftests/net/msg_zerocopy.sh |  5 ++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/msg_zerocopy.c b/tools/testing/selftests/net/msg_zerocopy.c
index 7ea5fb28c93d..20e334b25fbd 100644
--- a/tools/testing/selftests/net/msg_zerocopy.c
+++ b/tools/testing/selftests/net/msg_zerocopy.c
@@ -30,6 +30,7 @@
 #include <arpa/inet.h>
 #include <error.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <linux/errqueue.h>
 #include <linux/if_packet.h>
@@ -50,6 +51,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/ioctl.h>
+#include <sys/sendfile.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/time.h>
@@ -74,6 +76,14 @@
 #define MSG_ZEROCOPY	0x4000000
 #endif
 
+#ifndef SENDFILE_ZC
+#define SENDFILE_ZC (0x2)
+#endif
+
+#ifndef __NR_sendfile2
+#define __NR_sendfile2 467
+#endif
+
 static int  cfg_cork;
 static bool cfg_cork_mixed;
 static int  cfg_cpu		= -1;		/* default: pin to last cpu */
@@ -87,6 +97,8 @@ static int  cfg_verbose;
 static int  cfg_waittime_ms	= 500;
 static int  cfg_notification_limit = 32;
 static bool cfg_zerocopy;
+static bool cfg_sendfile;
+static const char *cfg_sendfile_path;
 
 static socklen_t cfg_alen;
 static struct sockaddr_storage cfg_dst_addr;
@@ -182,6 +194,37 @@ static void add_zcopy_cookie(struct msghdr *msg, uint32_t cookie)
 	memcpy(CMSG_DATA(cm), &cookie, sizeof(cookie));
 }
 
+static bool do_sendfile(int fd)
+{
+	int from_fd = open(cfg_sendfile_path, O_RDONLY, 0);
+	struct stat buf;
+	ssize_t total = 0;
+	ssize_t ret = 0;
+	off_t off = 0;
+
+	if (fd < 0)
+		error(1, errno, "couldn't open sendfile path");
+
+	if (fstat(from_fd, &buf))
+		error(1, errno, "couldn't fstat");
+
+	while (total < buf.st_size) {
+		ret = syscall(__NR_sendfile2, fd, from_fd, &off, buf.st_size,
+			      SENDFILE_ZC);
+		if (ret < 0)
+			error(1, errno, "unable to sendfile");
+		total += ret;
+		sends_since_notify++;
+		bytes += ret;
+		packets++;
+		if (ret > 0)
+			expected_completions++;
+	}
+
+	close(from_fd);
+	return total == buf.st_size;
+}
+
 static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
 {
 	int ret, len, i, flags;
@@ -550,6 +593,8 @@ static void do_tx(int domain, int type, int protocol)
 	do {
 		if (cfg_cork)
 			do_sendmsg_corked(fd, &msg);
+		else if (cfg_sendfile)
+			do_sendfile(fd);
 		else
 			do_sendmsg(fd, &msg, cfg_zerocopy, domain);
 
@@ -715,7 +760,7 @@ static void parse_opts(int argc, char **argv)
 
 	cfg_payload_len = max_payload_len;
 
-	while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vz")) != -1) {
+	while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vzf:w:")) != -1) {
 		switch (c) {
 		case '4':
 			if (cfg_family != PF_UNSPEC)
@@ -767,9 +812,16 @@ static void parse_opts(int argc, char **argv)
 		case 'v':
 			cfg_verbose++;
 			break;
+		case 'f':
+			cfg_sendfile = true;
+			cfg_sendfile_path = optarg;
+			break;
 		case 'z':
 			cfg_zerocopy = true;
 			break;
+		case 'w':
+			cfg_waittime_ms = 200 + strtoul(optarg, NULL, 10) * 1000;
+			break;
 		}
 	}
 
diff --git a/tools/testing/selftests/net/msg_zerocopy.sh b/tools/testing/selftests/net/msg_zerocopy.sh
index 89c22f5320e0..c735e4ab86b5 100755
--- a/tools/testing/selftests/net/msg_zerocopy.sh
+++ b/tools/testing/selftests/net/msg_zerocopy.sh
@@ -74,6 +74,7 @@ esac
 cleanup() {
 	ip netns del "${NS2}"
 	ip netns del "${NS1}"
+	rm -f sendfile_data
 }
 
 trap cleanup EXIT
@@ -106,6 +107,9 @@ ip -netns "${NS2}" addr add       fd::2/64 dev "${DEV}" nodad
 # Optionally disable sg or csum offload to test edge cases
 # ip netns exec "${NS1}" ethtool -K "${DEV}" sg off
 
+# create sendfile test data
+dd if=/dev/zero of=sendfile_data bs=1M count=8 2> /dev/null
+
 do_test() {
 	local readonly ARGS="$1"
 
@@ -118,4 +122,5 @@ do_test() {
 
 do_test "${EXTRA_ARGS}"
 do_test "-z ${EXTRA_ARGS}"
+do_test "-z -f sendfile_data ${EXTRA_ARGS}"
 echo ok
-- 
2.43.0


^ permalink raw reply related

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Christoph Hellwig @ 2025-03-19  8:04 UTC (permalink / raw)
  To: Joe Damato
  Cc: netdev, linux-kernel, asml.silence, linux-fsdevel, edumazet,
	pabeni, horms, linux-api, linux-arch, viro, jack, kuba, shuah,
	sdf, mingo, arnd, brauner, akpm, tglx, jolsa, linux-kselftest
In-Reply-To: <20250319001521.53249-1-jdamato@fastly.com>

On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
> One way to fix this is to add zerocopy notifications to sendfile similar
> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
> extensive work done by Pavel [1].

What is a "zerocopy notification" and why aren't you simply plugging
this into io_uring and generate a CQE so that it works like all other
asynchronous operations?


^ permalink raw reply

* Re: [PATCH v4 0/5] implement lightweight guard pages
From: Alexander Mikhalitsyn @ 2025-03-19 14:50 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: James.Bottomley, Liam.Howlett, akpm, arnd, brauner, chris, david,
	deller, hch, jannh, jcmvbkbc, jeffxu, jhubbard, linux-api,
	linux-kernel, linux-mm, mattst88, muchun.song, paulmck,
	richard.henderson, shuah, sidhartha.kumar, surenb, tsbogend,
	vbabka, willy, criu, Andrei Vagin, Pavel Tikhomirov
In-Reply-To: <cover.1730123433.git.lorenzo.stoakes@oracle.com>

On Mon, Oct 28, 2024 at 02:13:26PM +0000, Lorenzo Stoakes wrote:
> Userland library functions such as allocators and threading implementations
> often require regions of memory to act as 'guard pages' - mappings which,
> when accessed, result in a fatal signal being sent to the accessing
> process.
> 
> The current means by which these are implemented is via a PROT_NONE mmap()
> mapping, which provides the required semantics however incur an overhead of
> a VMA for each such region.
> 
> With a great many processes and threads, this can rapidly add up and incur
> a significant memory penalty. It also has the added problem of preventing
> merges that might otherwise be permitted.
> 
> This series takes a different approach - an idea suggested by Vlasimil
> Babka (and before him David Hildenbrand and Jann Horn - perhaps more - the
> provenance becomes a little tricky to ascertain after this - please forgive
> any omissions!)  - rather than locating the guard pages at the VMA layer,
> instead placing them in page tables mapping the required ranges.
> 
> Early testing of the prototype version of this code suggests a 5 times
> speed up in memory mapping invocations (in conjunction with use of
> process_madvise()) and a 13% reduction in VMAs on an entirely idle android
> system and unoptimised code.
> 
> We expect with optimisation and a loaded system with a larger number of
> guard pages this could significantly increase, but in any case these
> numbers are encouraging.
> 
> This way, rather than having separate VMAs specifying which parts of a
> range are guard pages, instead we have a VMA spanning the entire range of
> memory a user is permitted to access and including ranges which are to be
> 'guarded'.
> 
> After mapping this, a user can specify which parts of the range should
> result in a fatal signal when accessed.
> 
> By restricting the ability to specify guard pages to memory mapped by
> existing VMAs, we can rely on the mappings being torn down when the
> mappings are ultimately unmapped and everything works simply as if the
> memory were not faulted in, from the point of view of the containing VMAs.
> 
> This mechanism in effect poisons memory ranges similar to hardware memory
> poisoning, only it is an entirely software-controlled form of poisoning.
> 
> The mechanism is implemented via madvise() behaviour - MADV_GUARD_INSTALL
> which installs page table-level guard page markers - and
> MADV_GUARD_REMOVE - which clears them.
> 
> Guard markers can be installed across multiple VMAs and any existing
> mappings will be cleared, that is zapped, before installing the guard page
> markers in the page tables.
> 
> There is no concept of 'nested' guard markers, multiple attempts to install
> guard markers in a range will, after the first attempt, have no effect.
> 
> Importantly, removing guard markers over a range that contains both guard
> markers and ordinary backed memory has no effect on anything but the guard
> markers (including leaving huge pages un-split), so a user can safely
> remove guard markers over a range of memory leaving the rest intact.
> 
> The actual mechanism by which the page table entries are specified makes
> use of existing logic - PTE markers, which are used for the userfaultfd
> UFFDIO_POISON mechanism.
> 
> Unfortunately PTE_MARKER_POISONED is not suited for the guard page
> mechanism as it results in VM_FAULT_HWPOISON semantics in the fault
> handler, so we add our own specific PTE_MARKER_GUARD and adapt existing
> logic to handle it.
> 
> We also extend the generic page walk mechanism to allow for installation of
> PTEs (carefully restricted to memory management logic only to prevent
> unwanted abuse).
> 
> We ensure that zapping performed by MADV_DONTNEED and MADV_FREE do not
> remove guard markers, nor does forking (except when VM_WIPEONFORK is
> specified for a VMA which implies a total removal of memory
> characteristics).
> 
> It's important to note that the guard page implementation is emphatically
> NOT a security feature, so a user can remove the markers if they wish. We
> simply implement it in such a way as to provide the least surprising
> behaviour.
> 
> An extensive set of self-tests are provided which ensure behaviour is as
> expected and additionally self-documents expected behaviour of guard
> ranges.

Dear Lorenzo,
Dear colleagues,

sorry about raising an old thread.

It looks like this feature is now used in glibc [1]. And we noticed failures in CRIU [2]
CI on Fedora Rawhide userspace. Now a question is how we can properly detect such 
"guarded" pages from user space. As I can see from MADV_GUARD_INSTALL implementation,
it does not modify VMA flags anyhow, but only page tables. It means that /proc/<pid>/maps
and /proc/<pid>/smaps interfaces are useless in this case. (Please, correct me if I'm missing
anything here.)

I wonder if you have any ideas / suggestions regarding Checkpoint/Restore here. We (CRIU devs) are happy
to develop some patches to bring some uAPI to expose MADV_GUARDs, but before going into this we decided
to raise this question in LKML.

+CC criu@lists.linux.dev
+CC Andrei Vagin <avagin@gmail.com>
+CC Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

Kind regards,
Alex

[1] https://github.com/bminor/glibc/commit/a6fbe36b7f31292981422692236465ab56670ea9
[2] https://github.com/checkpoint-restore/criu/pull/2625

> 
> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Suggested-by: Jann Horn <jannh@google.com>
> Suggested-by: David Hildenbrand <david@redhat.com>
> 
> v4
> * Use restart_syscall() to implement -ERESTARTNOINTR to ensure correctly
>   handled by kernel - tested this code path and confirmed it works
>   correctly. Thanks to Vlastimil for pointing this issue out!
> * Updated the vector_madvise() handler to not unnecessarily invoke
>   cond_resched() as suggested by Vlastimil.
> * Updated guard page tests to add a test for a vector operation which
>   overwrites existing mappings. Tested this against the -ERESTARTNOINTR
>   case and confirmed working.
> * Improved page walk logic further, refactoring handling logic as suggested
>   by Vlastimil.
> * Moved MAX_MADVISE_GUARD_RETRIES to mm/madvise.c as suggested by Vlastimil.
> 
> v3
> * Cleaned up mm/pagewalk.c logic a bit to make things clearer, as suggested
>   by Vlastiml.
> * Explicitly avoid splitting THP on PTE installation, as suggested by
>   Vlastimil. Note this has no impact on the guard pages logic, which has
>   page table entry handlers at PUD, PMD and PTE level.
> * Added WARN_ON_ONCE() to mm/hugetlb.c path where we don't expect a guard
>   marker, as suggested by Vlastimil.
> * Reverted change to is_poisoned_swp_entry() to exclude guard pages which
>   has the effect of MADV_FREE _not_ clearing guard pages. After discussion
>   with Vlastimil, it became apparent that the ability to 'cancel' the
>   freeing operation by writing to the mapping after having issued an
>   MADV_FREE would mean that we would risk unexpected behaviour should the
>   guard pages be removed, so we now do not remove markers here at all.
> * Added comment to PTE_MARKER_GUARD to highlight that memory tagged with
>   the marker behaves as if it were a region mapped PROT_NONE, as
>   highlighted by David.
> * Rename poison -> install, unpoison -> remove (i.e. MADV_GUARD_INSTALL /
>   MADV_GUARD_REMOVE over MADV_GUARD_POISON / MADV_GUARD_REMOVE) at the
>   request of David and John who both find the poison analogy
>   confusing/overloaded.
> * After a lot of discussion, replace the looping behaviour should page
>   faults race with guard page installation with a modest reattempt followed
>   by returning -ERESTARTNOINTR to have the operation abort and re-enter,
>   relieving lock contention and avoiding the possibility of allowing a
>   malicious sandboxed process to impact the mmap lock or stall the overall
>   process more than necessary, as suggested by Jann and Vlastimil having
>   raised the issue.
> * Adjusted the page table walker so a populated huge PUD or PMD is
>   correctly treated as being populated, necessitating a zap. In v2 we
>   incorrectly skipped over these, which would cause the logic to wrongly
>   proceed as if nothing were populated and the install succeeded.
>   Instead, explicitly check to see if a huge page - if so, do not split but
>   rather abort the operation and let zap take care of things.
> * Updated the guard remove logic to not unnecessarily split huge pages
>   either.
> * Added a debug check to assert that the number of installed PTEs matches
>   expectation, accounting for any existing guard pages.
> * Adapted vector_madvise() used by the process_madvise() system call to
>   handle -ERESTARTNOINTR correctly.
> https://lore.kernel.org/all/cover.1729699916.git.lorenzo.stoakes@oracle.com/
> 
> v2
> * The macros in kselftest_harness.h seem to be broken - __EXPECT() is
>   terminated by '} while (0); OPTIONAL_HANDLER(_assert)' meaning it is not
>   safe in single line if / else or for /which blocks, however working
>   around this results in checkpatch producing invalid warnings, as reported
>   by Shuah.
> * Fixing these macros is out of scope for this series, so compromise and
>   instead rewrite test blocks so as to use multiple lines by separating out
>   a decl in most cases. This has the side effect of, for the most part,
>   making things more readable.
> * Heavily document the use of the volatile keyword - we can't avoid
>   checkpatch complaining about this, so we explain it, as reported by
>   Shuah.
> * Updated commit message to highlight that we skip tests we lack
>   permissions for, as reported by Shuah.
> * Replaced a perror() with ksft_exit_fail_perror(), as reported by Shuah.
> * Added user friendly messages to cases where tests are skipped due to lack
>   of permissions, as reported by Shuah.
> * Update the tool header to include the new MADV_GUARD_POISON/UNPOISON
>   defines and directly include asm-generic/mman.h to get the
>   platform-neutral versions to ensure we import them.
> * Finally fixed Vlastimil's email address in Suggested-by tags from suze to
>   suse, as reported by Vlastimil.
> * Added linux-api to cc list, as reported by Vlastimil.
> https://lore.kernel.org/all/cover.1729440856.git.lorenzo.stoakes@oracle.com/
> 
> v1
> * Un-RFC'd as appears no major objections to approach but rather debate on
>   implementation.
> * Fixed issue with arches which need mmu_context.h and
>   tlbfush.h. header imports in pagewalker logic to be able to use
>   update_mmu_cache() as reported by the kernel test bot.
> * Added comments in page walker logic to clarify who can use
>   ops->install_pte and why as well as adding a check_ops_valid() helper
>   function, as suggested by Christoph.
> * Pass false in full parameter in pte_clear_not_present_full() as suggested
>   by Jann.
> * Stopped erroneously requiring a write lock for the poison operation as
>   suggested by Jann and Suren.
> * Moved anon_vma_prepare() to the start of madvise_guard_poison() to be
>   consistent with how this is used elsewhere in the kernel as suggested by
>   Jann.
> * Avoid returning -EAGAIN if we are raced on page faults, just keep looping
>   and duck out if a fatal signal is pending or a conditional reschedule is
>   needed, as suggested by Jann.
> * Avoid needlessly splitting huge PUDs and PMDs by specifying
>   ACTION_CONTINUE, as suggested by Jann.
> https://lore.kernel.org/all/cover.1729196871.git.lorenzo.stoakes@oracle.com/
> 
> RFC
> https://lore.kernel.org/all/cover.1727440966.git.lorenzo.stoakes@oracle.com/
> 
> Lorenzo Stoakes (5):
>   mm: pagewalk: add the ability to install PTEs
>   mm: add PTE_MARKER_GUARD PTE marker
>   mm: madvise: implement lightweight guard page mechanism
>   tools: testing: update tools UAPI header for mman-common.h
>   selftests/mm: add self tests for guard page feature
> 
>  arch/alpha/include/uapi/asm/mman.h           |    3 +
>  arch/mips/include/uapi/asm/mman.h            |    3 +
>  arch/parisc/include/uapi/asm/mman.h          |    3 +
>  arch/xtensa/include/uapi/asm/mman.h          |    3 +
>  include/linux/mm_inline.h                    |    2 +-
>  include/linux/pagewalk.h                     |   18 +-
>  include/linux/swapops.h                      |   24 +-
>  include/uapi/asm-generic/mman-common.h       |    3 +
>  mm/hugetlb.c                                 |    4 +
>  mm/internal.h                                |    6 +
>  mm/madvise.c                                 |  239 ++++
>  mm/memory.c                                  |   18 +-
>  mm/mprotect.c                                |    6 +-
>  mm/mseal.c                                   |    1 +
>  mm/pagewalk.c                                |  246 +++-
>  tools/include/uapi/asm-generic/mman-common.h |    3 +
>  tools/testing/selftests/mm/.gitignore        |    1 +
>  tools/testing/selftests/mm/Makefile          |    1 +
>  tools/testing/selftests/mm/guard-pages.c     | 1243 ++++++++++++++++++
>  19 files changed, 1751 insertions(+), 76 deletions(-)
>  create mode 100644 tools/testing/selftests/mm/guard-pages.c
> 
> --
> 2.47.0

^ permalink raw reply

* Re: [PATCH v4 0/5] implement lightweight guard pages
From: David Hildenbrand @ 2025-03-19 14:52 UTC (permalink / raw)
  To: Alexander Mikhalitsyn, Lorenzo Stoakes
  Cc: James.Bottomley, Liam.Howlett, akpm, arnd, brauner, chris, deller,
	hch, jannh, jcmvbkbc, jeffxu, jhubbard, linux-api, linux-kernel,
	linux-mm, mattst88, muchun.song, paulmck, richard.henderson,
	shuah, sidhartha.kumar, surenb, tsbogend, vbabka, willy, criu,
	Andrei Vagin, Pavel Tikhomirov
In-Reply-To: <zihwmp67m2lpuxbfktmztvjdyap7suzd75dowlw4eamu6bhjf3@6euydiqowc7h>

On 19.03.25 15:50, Alexander Mikhalitsyn wrote:
> On Mon, Oct 28, 2024 at 02:13:26PM +0000, Lorenzo Stoakes wrote:
>> Userland library functions such as allocators and threading implementations
>> often require regions of memory to act as 'guard pages' - mappings which,
>> when accessed, result in a fatal signal being sent to the accessing
>> process.
>>
>> The current means by which these are implemented is via a PROT_NONE mmap()
>> mapping, which provides the required semantics however incur an overhead of
>> a VMA for each such region.
>>
>> With a great many processes and threads, this can rapidly add up and incur
>> a significant memory penalty. It also has the added problem of preventing
>> merges that might otherwise be permitted.
>>
>> This series takes a different approach - an idea suggested by Vlasimil
>> Babka (and before him David Hildenbrand and Jann Horn - perhaps more - the
>> provenance becomes a little tricky to ascertain after this - please forgive
>> any omissions!)  - rather than locating the guard pages at the VMA layer,
>> instead placing them in page tables mapping the required ranges.
>>
>> Early testing of the prototype version of this code suggests a 5 times
>> speed up in memory mapping invocations (in conjunction with use of
>> process_madvise()) and a 13% reduction in VMAs on an entirely idle android
>> system and unoptimised code.
>>
>> We expect with optimisation and a loaded system with a larger number of
>> guard pages this could significantly increase, but in any case these
>> numbers are encouraging.
>>
>> This way, rather than having separate VMAs specifying which parts of a
>> range are guard pages, instead we have a VMA spanning the entire range of
>> memory a user is permitted to access and including ranges which are to be
>> 'guarded'.
>>
>> After mapping this, a user can specify which parts of the range should
>> result in a fatal signal when accessed.
>>
>> By restricting the ability to specify guard pages to memory mapped by
>> existing VMAs, we can rely on the mappings being torn down when the
>> mappings are ultimately unmapped and everything works simply as if the
>> memory were not faulted in, from the point of view of the containing VMAs.
>>
>> This mechanism in effect poisons memory ranges similar to hardware memory
>> poisoning, only it is an entirely software-controlled form of poisoning.
>>
>> The mechanism is implemented via madvise() behaviour - MADV_GUARD_INSTALL
>> which installs page table-level guard page markers - and
>> MADV_GUARD_REMOVE - which clears them.
>>
>> Guard markers can be installed across multiple VMAs and any existing
>> mappings will be cleared, that is zapped, before installing the guard page
>> markers in the page tables.
>>
>> There is no concept of 'nested' guard markers, multiple attempts to install
>> guard markers in a range will, after the first attempt, have no effect.
>>
>> Importantly, removing guard markers over a range that contains both guard
>> markers and ordinary backed memory has no effect on anything but the guard
>> markers (including leaving huge pages un-split), so a user can safely
>> remove guard markers over a range of memory leaving the rest intact.
>>
>> The actual mechanism by which the page table entries are specified makes
>> use of existing logic - PTE markers, which are used for the userfaultfd
>> UFFDIO_POISON mechanism.
>>
>> Unfortunately PTE_MARKER_POISONED is not suited for the guard page
>> mechanism as it results in VM_FAULT_HWPOISON semantics in the fault
>> handler, so we add our own specific PTE_MARKER_GUARD and adapt existing
>> logic to handle it.
>>
>> We also extend the generic page walk mechanism to allow for installation of
>> PTEs (carefully restricted to memory management logic only to prevent
>> unwanted abuse).
>>
>> We ensure that zapping performed by MADV_DONTNEED and MADV_FREE do not
>> remove guard markers, nor does forking (except when VM_WIPEONFORK is
>> specified for a VMA which implies a total removal of memory
>> characteristics).
>>
>> It's important to note that the guard page implementation is emphatically
>> NOT a security feature, so a user can remove the markers if they wish. We
>> simply implement it in such a way as to provide the least surprising
>> behaviour.
>>
>> An extensive set of self-tests are provided which ensure behaviour is as
>> expected and additionally self-documents expected behaviour of guard
>> ranges.
> 
> Dear Lorenzo,
> Dear colleagues,
> 
> sorry about raising an old thread.
> 
> It looks like this feature is now used in glibc [1]. And we noticed failures in CRIU [2]
> CI on Fedora Rawhide userspace. Now a question is how we can properly detect such
> "guarded" pages from user space. As I can see from MADV_GUARD_INSTALL implementation,
> it does not modify VMA flags anyhow, but only page tables. It means that /proc/<pid>/maps
> and /proc/<pid>/smaps interfaces are useless in this case. (Please, correct me if I'm missing
> anything here.)
> 
> I wonder if you have any ideas / suggestions regarding Checkpoint/Restore here. We (CRIU devs) are happy
> to develop some patches to bring some uAPI to expose MADV_GUARDs, but before going into this we decided
> to raise this question in LKML.


See [1] and [2]

[1] 
https://lkml.kernel.org/r/cover.1740139449.git.lorenzo.stoakes@oracle.com
[2] https://lwn.net/Articles/1011366/


-- 
Cheers,

David / dhildenb


^ permalink raw reply

* Re: [PATCH v4 0/5] implement lightweight guard pages
From: Lorenzo Stoakes @ 2025-03-19 15:02 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Alexander Mikhalitsyn, James.Bottomley, Liam.Howlett, akpm, arnd,
	brauner, chris, deller, hch, jannh, jcmvbkbc, jeffxu, jhubbard,
	linux-api, linux-kernel, linux-mm, mattst88, muchun.song, paulmck,
	richard.henderson, shuah, sidhartha.kumar, surenb, tsbogend,
	vbabka, willy, criu, Andrei Vagin, Pavel Tikhomirov
In-Reply-To: <278393de-2729-4ed0-822c-87f33c7ce27e@redhat.com>

On Wed, Mar 19, 2025 at 03:52:56PM +0100, David Hildenbrand wrote:
> On 19.03.25 15:50, Alexander Mikhalitsyn wrote:
> > On Mon, Oct 28, 2024 at 02:13:26PM +0000, Lorenzo Stoakes wrote:

[snip]


> >
> > Dear Lorenzo,
> > Dear colleagues,
> >
> > sorry about raising an old thread.
> >

No worries!

> > It looks like this feature is now used in glibc [1]. And we noticed failures in CRIU [2]
> > CI on Fedora Rawhide userspace. Now a question is how we can properly detect such
> > "guarded" pages from user space. As I can see from MADV_GUARD_INSTALL implementation,
> > it does not modify VMA flags anyhow, but only page tables. It means that /proc/<pid>/maps
> > and /proc/<pid>/smaps interfaces are useless in this case. (Please, correct me if I'm missing
> > anything here.)

Sorry to hear that.

> >
> > I wonder if you have any ideas / suggestions regarding Checkpoint/Restore here. We (CRIU devs) are happy
> > to develop some patches to bring some uAPI to expose MADV_GUARDs, but before going into this we decided
> > to raise this question in LKML.

There's no need.

>
>
> See [1] and [2]
>
> [1]
> https://lkml.kernel.org/r/cover.1740139449.git.lorenzo.stoakes@oracle.com
> [2] https://lwn.net/Articles/1011366/

As per David, there is already a feature heading for 6.15 which will allow
this to be exposed by /proc/$pid/pagemap.

In addition, I plan to add a 'maybe has guard regions' flag that can be
observed in smaps to assist narrowing down which VMAs to check.

However unfortunately due to the nature of the feature there is no getting
around the need to traverse page tables.

That thread (and LWN article :) go into extensive detail as to why. In
essence - it's the basis of its design to express this information at the
page table level only, and any attempt to encode this at the VMA level
(other than a 'maybe' flag) would eliminate the purpose of the feature.

Let me know if there's any way I can help!

Cheers, Lorenzo

>
>
> --
> Cheers,
>
> David / dhildenb
>

^ permalink raw reply

* Re: [PATCH v4 0/5] implement lightweight guard pages
From: Aleksandr Mikhalitsyn @ 2025-03-19 15:08 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Lorenzo Stoakes, James.Bottomley, Liam.Howlett, akpm, arnd,
	brauner, chris, deller, hch, jannh, jcmvbkbc, jeffxu, jhubbard,
	linux-api, linux-kernel, linux-mm, mattst88, muchun.song, paulmck,
	richard.henderson, shuah, sidhartha.kumar, surenb, tsbogend,
	vbabka, willy, criu, Andrei Vagin, Pavel Tikhomirov
In-Reply-To: <278393de-2729-4ed0-822c-87f33c7ce27e@redhat.com>

On Wed, Mar 19, 2025 at 3:53 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 19.03.25 15:50, Alexander Mikhalitsyn wrote:
> > On Mon, Oct 28, 2024 at 02:13:26PM +0000, Lorenzo Stoakes wrote:
> >> Userland library functions such as allocators and threading implementations
> >> often require regions of memory to act as 'guard pages' - mappings which,
> >> when accessed, result in a fatal signal being sent to the accessing
> >> process.
> >>
> >> The current means by which these are implemented is via a PROT_NONE mmap()
> >> mapping, which provides the required semantics however incur an overhead of
> >> a VMA for each such region.
> >>
> >> With a great many processes and threads, this can rapidly add up and incur
> >> a significant memory penalty. It also has the added problem of preventing
> >> merges that might otherwise be permitted.
> >>
> >> This series takes a different approach - an idea suggested by Vlasimil
> >> Babka (and before him David Hildenbrand and Jann Horn - perhaps more - the
> >> provenance becomes a little tricky to ascertain after this - please forgive
> >> any omissions!)  - rather than locating the guard pages at the VMA layer,
> >> instead placing them in page tables mapping the required ranges.
> >>
> >> Early testing of the prototype version of this code suggests a 5 times
> >> speed up in memory mapping invocations (in conjunction with use of
> >> process_madvise()) and a 13% reduction in VMAs on an entirely idle android
> >> system and unoptimised code.
> >>
> >> We expect with optimisation and a loaded system with a larger number of
> >> guard pages this could significantly increase, but in any case these
> >> numbers are encouraging.
> >>
> >> This way, rather than having separate VMAs specifying which parts of a
> >> range are guard pages, instead we have a VMA spanning the entire range of
> >> memory a user is permitted to access and including ranges which are to be
> >> 'guarded'.
> >>
> >> After mapping this, a user can specify which parts of the range should
> >> result in a fatal signal when accessed.
> >>
> >> By restricting the ability to specify guard pages to memory mapped by
> >> existing VMAs, we can rely on the mappings being torn down when the
> >> mappings are ultimately unmapped and everything works simply as if the
> >> memory were not faulted in, from the point of view of the containing VMAs.
> >>
> >> This mechanism in effect poisons memory ranges similar to hardware memory
> >> poisoning, only it is an entirely software-controlled form of poisoning.
> >>
> >> The mechanism is implemented via madvise() behaviour - MADV_GUARD_INSTALL
> >> which installs page table-level guard page markers - and
> >> MADV_GUARD_REMOVE - which clears them.
> >>
> >> Guard markers can be installed across multiple VMAs and any existing
> >> mappings will be cleared, that is zapped, before installing the guard page
> >> markers in the page tables.
> >>
> >> There is no concept of 'nested' guard markers, multiple attempts to install
> >> guard markers in a range will, after the first attempt, have no effect.
> >>
> >> Importantly, removing guard markers over a range that contains both guard
> >> markers and ordinary backed memory has no effect on anything but the guard
> >> markers (including leaving huge pages un-split), so a user can safely
> >> remove guard markers over a range of memory leaving the rest intact.
> >>
> >> The actual mechanism by which the page table entries are specified makes
> >> use of existing logic - PTE markers, which are used for the userfaultfd
> >> UFFDIO_POISON mechanism.
> >>
> >> Unfortunately PTE_MARKER_POISONED is not suited for the guard page
> >> mechanism as it results in VM_FAULT_HWPOISON semantics in the fault
> >> handler, so we add our own specific PTE_MARKER_GUARD and adapt existing
> >> logic to handle it.
> >>
> >> We also extend the generic page walk mechanism to allow for installation of
> >> PTEs (carefully restricted to memory management logic only to prevent
> >> unwanted abuse).
> >>
> >> We ensure that zapping performed by MADV_DONTNEED and MADV_FREE do not
> >> remove guard markers, nor does forking (except when VM_WIPEONFORK is
> >> specified for a VMA which implies a total removal of memory
> >> characteristics).
> >>
> >> It's important to note that the guard page implementation is emphatically
> >> NOT a security feature, so a user can remove the markers if they wish. We
> >> simply implement it in such a way as to provide the least surprising
> >> behaviour.
> >>
> >> An extensive set of self-tests are provided which ensure behaviour is as
> >> expected and additionally self-documents expected behaviour of guard
> >> ranges.
> >
> > Dear Lorenzo,
> > Dear colleagues,
> >
> > sorry about raising an old thread.
> >
> > It looks like this feature is now used in glibc [1]. And we noticed failures in CRIU [2]
> > CI on Fedora Rawhide userspace. Now a question is how we can properly detect such
> > "guarded" pages from user space. As I can see from MADV_GUARD_INSTALL implementation,
> > it does not modify VMA flags anyhow, but only page tables. It means that /proc/<pid>/maps
> > and /proc/<pid>/smaps interfaces are useless in this case. (Please, correct me if I'm missing
> > anything here.)
> >
> > I wonder if you have any ideas / suggestions regarding Checkpoint/Restore here. We (CRIU devs) are happy
> > to develop some patches to bring some uAPI to expose MADV_GUARDs, but before going into this we decided
> > to raise this question in LKML.
>
>
> See [1] and [2]

Hi David,

Huge thanks for such a fast and helpful reply ;)

>
> [1]
> https://lkml.kernel.org/r/cover.1740139449.git.lorenzo.stoakes@oracle.com
> [2] https://lwn.net/Articles/1011366/
>
>
> --
> Cheers,
>
> David / dhildenb

Kind regards,
Alex

>

^ permalink raw reply

* Re: [PATCH v4 0/5] implement lightweight guard pages
From: Aleksandr Mikhalitsyn @ 2025-03-19 15:15 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: David Hildenbrand, James.Bottomley, Liam.Howlett, akpm, arnd,
	brauner, chris, deller, hch, jannh, jcmvbkbc, jeffxu, jhubbard,
	linux-api, linux-kernel, linux-mm, mattst88, muchun.song, paulmck,
	richard.henderson, shuah, sidhartha.kumar, surenb, tsbogend,
	vbabka, willy, criu, Andrei Vagin, Pavel Tikhomirov
In-Reply-To: <23000b6a-8a58-4c38-a032-ad62637d3fa4@lucifer.local>

On Wed, Mar 19, 2025 at 4:02 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Wed, Mar 19, 2025 at 03:52:56PM +0100, David Hildenbrand wrote:
> > On 19.03.25 15:50, Alexander Mikhalitsyn wrote:
> > > On Mon, Oct 28, 2024 at 02:13:26PM +0000, Lorenzo Stoakes wrote:
>
> [snip]
>
>
> > >
> > > Dear Lorenzo,
> > > Dear colleagues,
> > >
> > > sorry about raising an old thread.
> > >
>
> No worries!
>
> > > It looks like this feature is now used in glibc [1]. And we noticed failures in CRIU [2]
> > > CI on Fedora Rawhide userspace. Now a question is how we can properly detect such
> > > "guarded" pages from user space. As I can see from MADV_GUARD_INSTALL implementation,
> > > it does not modify VMA flags anyhow, but only page tables. It means that /proc/<pid>/maps
> > > and /proc/<pid>/smaps interfaces are useless in this case. (Please, correct me if I'm missing
> > > anything here.)
>
> Sorry to hear that.

No problem at all ;)

>
> > >
> > > I wonder if you have any ideas / suggestions regarding Checkpoint/Restore here. We (CRIU devs) are happy
> > > to develop some patches to bring some uAPI to expose MADV_GUARDs, but before going into this we decided
> > > to raise this question in LKML.
>
> There's no need.
>
> >
> >
> > See [1] and [2]
> >
> > [1]
> > https://lkml.kernel.org/r/cover.1740139449.git.lorenzo.stoakes@oracle.com
> > [2] https://lwn.net/Articles/1011366/
>
> As per David, there is already a feature heading for 6.15 which will allow
> this to be exposed by /proc/$pid/pagemap.

Yeah, that's indeed very helpful!

>
> In addition, I plan to add a 'maybe has guard regions' flag that can be
> observed in smaps to assist narrowing down which VMAs to check.
>
> However unfortunately due to the nature of the feature there is no getting
> around the need to traverse page tables.
>
> That thread (and LWN article :) go into extensive detail as to why. In
> essence - it's the basis of its design to express this information at the
> page table level only, and any attempt to encode this at the VMA level
> (other than a 'maybe' flag) would eliminate the purpose of the feature.

Thank you very much for these explanations!
I'll read the LWN article too.

>
> Let me know if there's any way I can help!

Keep in contact! ;)

Kind regards,
Alex

>
> Cheers, Lorenzo
>
> >
> >
> > --
> > Cheers,
> >
> > David / dhildenb
> >

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-19 15:32 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: netdev, linux-kernel, asml.silence, linux-fsdevel, edumazet,
	pabeni, horms, linux-api, linux-arch, viro, jack, kuba, shuah,
	sdf, mingo, arnd, brauner, akpm, tglx, jolsa, linux-kselftest
In-Reply-To: <Z9p6oFlHxkYvUA8N@infradead.org>

On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
> > One way to fix this is to add zerocopy notifications to sendfile similar
> > to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
> > extensive work done by Pavel [1].
> 
> What is a "zerocopy notification" 

See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
sendmsg and passes MSG_ZEROCOPY a completion notification is added
to the error queue. The user app can poll for these to find out when
the TX has completed and the buffer it passed to the kernel can be
overwritten.

My series provides the same functionality via splice and sendfile2.

[1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html

> and why aren't you simply plugging this into io_uring and generate
> a CQE so that it works like all other asynchronous operations?

I linked to the iouring work that Pavel did in the cover letter.
Please take a look.

That work refactored the internals of how zerocopy completion
notifications are wired up, allowing other pieces of code to use the
same infrastructure and extend it, if needed.

My series is using the same internals that iouring (and others) use
to generate zerocopy completion notifications. Unlike iouring,
though, I don't need a fully customized implementation with a new
user API for harvesting completion events; I can use the existing
mechanism already in the kernel that user apps already use for
sendmsg (the error queue, as explained above and in the
MSG_ZEROCOPY documentation).

Let me know if that answers your question or if you have other
questions.

Thanks,
Joe

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-19 16:07 UTC (permalink / raw)
  To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <Z9rjgyl7_61Ddzrq@LQ3V64L9R2>

On 3/19/25 9:32 AM, Joe Damato wrote:
> On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
>> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
>>> One way to fix this is to add zerocopy notifications to sendfile similar
>>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
>>> extensive work done by Pavel [1].
>>
>> What is a "zerocopy notification" 
> 
> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> sendmsg and passes MSG_ZEROCOPY a completion notification is added
> to the error queue. The user app can poll for these to find out when
> the TX has completed and the buffer it passed to the kernel can be
> overwritten.
> 
> My series provides the same functionality via splice and sendfile2.
> 
> [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
> 
>> and why aren't you simply plugging this into io_uring and generate
>> a CQE so that it works like all other asynchronous operations?
> 
> I linked to the iouring work that Pavel did in the cover letter.
> Please take a look.
> 
> That work refactored the internals of how zerocopy completion
> notifications are wired up, allowing other pieces of code to use the
> same infrastructure and extend it, if needed.
> 
> My series is using the same internals that iouring (and others) use
> to generate zerocopy completion notifications. Unlike iouring,
> though, I don't need a fully customized implementation with a new
> user API for harvesting completion events; I can use the existing
> mechanism already in the kernel that user apps already use for
> sendmsg (the error queue, as explained above and in the
> MSG_ZEROCOPY documentation).

The error queue is arguably a work-around for _not_ having a delivery
mechanism that works with a sync syscall in the first place. The main
question here imho would be "why add a whole new syscall etc when
there's already an existing way to do accomplish this, with
free-to-reuse notifications". If the answer is "because splice", then it
would seem saner to plumb up those bits only. Would be much simpler
too...

-- 
Jens Axboe

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-19 17:04 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <2d68bc91-c22c-4b48-a06d-fa9ec06dfb25@kernel.dk>

On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
> On 3/19/25 9:32 AM, Joe Damato wrote:
> > On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
> >> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
> >>> One way to fix this is to add zerocopy notifications to sendfile similar
> >>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
> >>> extensive work done by Pavel [1].
> >>
> >> What is a "zerocopy notification" 
> > 
> > See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> > sendmsg and passes MSG_ZEROCOPY a completion notification is added
> > to the error queue. The user app can poll for these to find out when
> > the TX has completed and the buffer it passed to the kernel can be
> > overwritten.
> > 
> > My series provides the same functionality via splice and sendfile2.
> > 
> > [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
> > 
> >> and why aren't you simply plugging this into io_uring and generate
> >> a CQE so that it works like all other asynchronous operations?
> > 
> > I linked to the iouring work that Pavel did in the cover letter.
> > Please take a look.
> > 
> > That work refactored the internals of how zerocopy completion
> > notifications are wired up, allowing other pieces of code to use the
> > same infrastructure and extend it, if needed.
> > 
> > My series is using the same internals that iouring (and others) use
> > to generate zerocopy completion notifications. Unlike iouring,
> > though, I don't need a fully customized implementation with a new
> > user API for harvesting completion events; I can use the existing
> > mechanism already in the kernel that user apps already use for
> > sendmsg (the error queue, as explained above and in the
> > MSG_ZEROCOPY documentation).
> 
> The error queue is arguably a work-around for _not_ having a delivery
> mechanism that works with a sync syscall in the first place. The main
> question here imho would be "why add a whole new syscall etc when
> there's already an existing way to do accomplish this, with
> free-to-reuse notifications". If the answer is "because splice", then it
> would seem saner to plumb up those bits only. Would be much simpler
> too...

I may be misunderstanding your comment, but my response would be:

  There are existing apps which use sendfile today unsafely and
  it would be very nice to have a safe sendfile equivalent. Converting
  existing apps to using iouring (if I understood your suggestion?)
  would be significantly more work compared to calling sendfile2 and
  adding code to check the error queue.

I would also argue that there are likely user apps out there that
use both sendmsg MSG_ZEROCOPY for certain writes (for data in
memory) and also use sendfile (for data on disk). One example would
be a reverse proxy that might write HTTP headers to clients via
sendmsg but transmit the response body with sendfile.

For those apps, the code to check the error queue already exists for
sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
way to ensure safe sendfile usage.

As far as the bit about plumbing only the splice bits, sorry if I'm
being dense here, do you mean plumbing the error queue through to
splice only and dropping sendfile2?

That is an option. Then the apps currently using sendfile could use
splice instead and get completion notifications on the error queue.
That would probably work and be less work than rewriting to use
iouring, but probably a bit more work than using a new syscall.

Thanks for taking a look and responding.

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-19 17:20 UTC (permalink / raw)
  To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <Z9r5JE3AJdnsXy_u@LQ3V64L9R2>

On 3/19/25 11:04 AM, Joe Damato wrote:
> On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
>> On 3/19/25 9:32 AM, Joe Damato wrote:
>>> On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
>>>> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
>>>>> One way to fix this is to add zerocopy notifications to sendfile similar
>>>>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
>>>>> extensive work done by Pavel [1].
>>>>
>>>> What is a "zerocopy notification" 
>>>
>>> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
>>> sendmsg and passes MSG_ZEROCOPY a completion notification is added
>>> to the error queue. The user app can poll for these to find out when
>>> the TX has completed and the buffer it passed to the kernel can be
>>> overwritten.
>>>
>>> My series provides the same functionality via splice and sendfile2.
>>>
>>> [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
>>>
>>>> and why aren't you simply plugging this into io_uring and generate
>>>> a CQE so that it works like all other asynchronous operations?
>>>
>>> I linked to the iouring work that Pavel did in the cover letter.
>>> Please take a look.
>>>
>>> That work refactored the internals of how zerocopy completion
>>> notifications are wired up, allowing other pieces of code to use the
>>> same infrastructure and extend it, if needed.
>>>
>>> My series is using the same internals that iouring (and others) use
>>> to generate zerocopy completion notifications. Unlike iouring,
>>> though, I don't need a fully customized implementation with a new
>>> user API for harvesting completion events; I can use the existing
>>> mechanism already in the kernel that user apps already use for
>>> sendmsg (the error queue, as explained above and in the
>>> MSG_ZEROCOPY documentation).
>>
>> The error queue is arguably a work-around for _not_ having a delivery
>> mechanism that works with a sync syscall in the first place. The main
>> question here imho would be "why add a whole new syscall etc when
>> there's already an existing way to do accomplish this, with
>> free-to-reuse notifications". If the answer is "because splice", then it
>> would seem saner to plumb up those bits only. Would be much simpler
>> too...
> 
> I may be misunderstanding your comment, but my response would be:
> 
>   There are existing apps which use sendfile today unsafely and
>   it would be very nice to have a safe sendfile equivalent. Converting
>   existing apps to using iouring (if I understood your suggestion?)
>   would be significantly more work compared to calling sendfile2 and
>   adding code to check the error queue.

It's really not, if you just want to use it as a sync kind of thing. If
you want to have multiple things in flight etc, yeah it could be more
work, you'd also get better performance that way. And you could use
things like registered buffers for either of them, which again would
likely make it more efficient.

If you just use it as a sync thing, it'd be pretty trivial to just wrap
a my_sendfile_foo() in a submit_and_wait operation, which issues and
waits on the completion in a single syscall. And if you want to wait on
the notification too, you could even do that in the same syscall and
wait on 2 CQEs. That'd be a downright trivial way to provide a sync way
of doing the same thing.

> I would also argue that there are likely user apps out there that
> use both sendmsg MSG_ZEROCOPY for certain writes (for data in
> memory) and also use sendfile (for data on disk). One example would
> be a reverse proxy that might write HTTP headers to clients via
> sendmsg but transmit the response body with sendfile.
> 
> For those apps, the code to check the error queue already exists for
> sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
> way to ensure safe sendfile usage.

Sure that is certainly possible. I didn't say that wasn't the case,
rather that the error queue approach is a work-around in the first place
for not having some kind of async notification mechanism for when it's
free to reuse.

> As far as the bit about plumbing only the splice bits, sorry if I'm
> being dense here, do you mean plumbing the error queue through to
> splice only and dropping sendfile2?
> 
> That is an option. Then the apps currently using sendfile could use
> splice instead and get completion notifications on the error queue.
> That would probably work and be less work than rewriting to use
> iouring, but probably a bit more work than using a new syscall.

Yep

-- 
Jens Axboe

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-19 17:45 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <19e3056c-2f7b-4f41-9c40-98955c4a9ed3@kernel.dk>

On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
> On 3/19/25 11:04 AM, Joe Damato wrote:
> > On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
> >> On 3/19/25 9:32 AM, Joe Damato wrote:
> >>> On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
> >>>> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
> >>>>> One way to fix this is to add zerocopy notifications to sendfile similar
> >>>>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
> >>>>> extensive work done by Pavel [1].
> >>>>
> >>>> What is a "zerocopy notification" 
> >>>
> >>> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> >>> sendmsg and passes MSG_ZEROCOPY a completion notification is added
> >>> to the error queue. The user app can poll for these to find out when
> >>> the TX has completed and the buffer it passed to the kernel can be
> >>> overwritten.
> >>>
> >>> My series provides the same functionality via splice and sendfile2.
> >>>
> >>> [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
> >>>
> >>>> and why aren't you simply plugging this into io_uring and generate
> >>>> a CQE so that it works like all other asynchronous operations?
> >>>
> >>> I linked to the iouring work that Pavel did in the cover letter.
> >>> Please take a look.
> >>>
> >>> That work refactored the internals of how zerocopy completion
> >>> notifications are wired up, allowing other pieces of code to use the
> >>> same infrastructure and extend it, if needed.
> >>>
> >>> My series is using the same internals that iouring (and others) use
> >>> to generate zerocopy completion notifications. Unlike iouring,
> >>> though, I don't need a fully customized implementation with a new
> >>> user API for harvesting completion events; I can use the existing
> >>> mechanism already in the kernel that user apps already use for
> >>> sendmsg (the error queue, as explained above and in the
> >>> MSG_ZEROCOPY documentation).
> >>
> >> The error queue is arguably a work-around for _not_ having a delivery
> >> mechanism that works with a sync syscall in the first place. The main
> >> question here imho would be "why add a whole new syscall etc when
> >> there's already an existing way to do accomplish this, with
> >> free-to-reuse notifications". If the answer is "because splice", then it
> >> would seem saner to plumb up those bits only. Would be much simpler
> >> too...
> > 
> > I may be misunderstanding your comment, but my response would be:
> > 
> >   There are existing apps which use sendfile today unsafely and
> >   it would be very nice to have a safe sendfile equivalent. Converting
> >   existing apps to using iouring (if I understood your suggestion?)
> >   would be significantly more work compared to calling sendfile2 and
> >   adding code to check the error queue.
> 
> It's really not, if you just want to use it as a sync kind of thing. If
> you want to have multiple things in flight etc, yeah it could be more
> work, you'd also get better performance that way. And you could use
> things like registered buffers for either of them, which again would
> likely make it more efficient.

I haven't argued that performance would be better using sendfile2
compared to iouring, just that existing apps which already use
sendfile (but do so unsafely) would probably be more likely to use a
safe alternative with existing examples of how to harvest completion
notifications vs something more complex, like wrapping iouring.

> If you just use it as a sync thing, it'd be pretty trivial to just wrap
> a my_sendfile_foo() in a submit_and_wait operation, which issues and
> waits on the completion in a single syscall. And if you want to wait on
> the notification too, you could even do that in the same syscall and
> wait on 2 CQEs. That'd be a downright trivial way to provide a sync way
> of doing the same thing.

I don't disagree; I just don't know if app developers:
  a.) know that this is possible to do, and
  b.) know how to do it

In general: it does seem a bit odd to me that there isn't a safe
sendfile syscall in Linux that uses existing completion notification
mechanisms.

> > I would also argue that there are likely user apps out there that
> > use both sendmsg MSG_ZEROCOPY for certain writes (for data in
> > memory) and also use sendfile (for data on disk). One example would
> > be a reverse proxy that might write HTTP headers to clients via
> > sendmsg but transmit the response body with sendfile.
> > 
> > For those apps, the code to check the error queue already exists for
> > sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
> > way to ensure safe sendfile usage.
> 
> Sure that is certainly possible. I didn't say that wasn't the case,
> rather that the error queue approach is a work-around in the first place
> for not having some kind of async notification mechanism for when it's
> free to reuse.

Of course, I certainly agree that the error queue is a work around.
But it works, app use it, and its fairly well known. I don't see any
reason, other than historical context, why sendmsg can use this
mechanism, splice can, but sendfile shouldn't?

> > As far as the bit about plumbing only the splice bits, sorry if I'm
> > being dense here, do you mean plumbing the error queue through to
> > splice only and dropping sendfile2?
> > 
> > That is an option. Then the apps currently using sendfile could use
> > splice instead and get completion notifications on the error queue.
> > That would probably work and be less work than rewriting to use
> > iouring, but probably a bit more work than using a new syscall.
> 
> Yep

I'm not opposed to dropping the sendfile2 part of the series for the
official submission. I do think it is a bit odd to add the
functionality to splice only, though, when probably many apps are
using splice via calls to sendfile and there is no way to safely use
sendfile.

If you feel very strongly that this cannot be merged without
dropping sendfile2 and only plumbing this through for splice, then
I'll drop the sendfile2 syscall when I submit officially (probably
next week?).

I do feel pretty strongly that it's more likely apps would use
sendfile2 and we'd have safer apps out in the wild. But, I could be
wrong.

That said: if the new syscsall is the blocker, I'll drop it and
offer a change to the sendfile man page suggesting users swap it
with calls to splice + error queue for safety.

I greatly appreciate you taking a look and your feedback.

Thanks,
Joe

^ permalink raw reply

* Re: [PATCH 1/2] fs/proc/task_mmu: add guard region bit to pagemap
From: Andrei Vagin @ 2025-03-19 18:22 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Lorenzo Stoakes, Andrew Morton, Jonathan Corbet, Shuah Khan,
	Suren Baghdasaryan, Kalesh Singh, Liam R . Howlett,
	Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
	Juan Yescas, linux-mm, linux-doc, linux-kernel, linux-fsdevel,
	linux-kselftest, linux-api, criu, Alexander Mikhalitsyn,
	Pavel Tikhomirov, Mike Rapoport
In-Reply-To: <09d7ca19-e6cc-4aa9-8474-8975373bdebd@redhat.com>

On Mon, Feb 24, 2025 at 2:39 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 24.02.25 11:18, Lorenzo Stoakes wrote:
> > On Mon, Feb 24, 2025 at 10:27:28AM +0100, David Hildenbrand wrote:
> >> On 21.02.25 13:05, Lorenzo Stoakes wrote:
> >>> Currently there is no means by which users can determine whether a given
> >>> page in memory is in fact a guard region, that is having had the
> >>> MADV_GUARD_INSTALL madvise() flag applied to it.
> >>>
> >>> This is intentional, as to provide this information in VMA metadata would
> >>> contradict the intent of the feature (providing a means to change fault
> >>> behaviour at a page table level rather than a VMA level), and would require
> >>> VMA metadata operations to scan page tables, which is unacceptable.
> >>>
> >>> In many cases, users have no need to reflect and determine what regions
> >>> have been designated guard regions, as it is the user who has established
> >>> them in the first place.
> >>>
> >>> But in some instances, such as monitoring software, or software that relies
> >>> upon being able to ascertain the nature of mappings within a remote process
> >>> for instance, it becomes useful to be able to determine which pages have
> >>> the guard region marker applied.
> >>>
> >>> This patch makes use of an unused pagemap bit (58) to provide this
> >>> information.
> >>>
> >>> This patch updates the documentation at the same time as making the change
> >>> such that the implementation of the feature and the documentation of it are
> >>> tied together.
> >>>
> >>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >>> ---
> >>
> >>
> >> Acked-by: David Hildenbrand <david@redhat.com>
> >
> > Thanks! :)
> >>
> >> Something that might be interesting is also extending the PAGEMAP_SCAN
> >> ioctl.
> >
> > Yeah, funny you should mention that, I did see that, but on reading the man
> > page it struck me that it requires the region to be uffd afaict? All the
> > tests seem to establish uffd, and the man page implies it:
> >
> >         To start tracking the written state (flag) of a page or range of
> >         memory, the UFFD_FEATURE_WP_ASYNC must be enabled by UFFDIO_API
> >         ioctl(2) on userfaultfd and memory range must be registered with
> >         UFFDIO_REGISTER ioctl(2) in UFFDIO_REGISTER_MODE_WP mode.
> >
> > It would be a bit of a weird edge case to add support there. I was excited
> > when I first saw this ioctl, then disappointed afterwards... but maybe I
> > got it wrong?

> >
>
> I never managed to review that fully, but I thing that
> UFFD_FEATURE_WP_ASYNC thingy is only required for PM_SCAN_CHECK_WPASYNC
> and PM_SCAN_WP_MATCHING.
>
> See pagemap_scan_test_walk().
>
> I do recall that it works on any VMA.
>
> Ah yes, tools/testing/selftests/mm/vm_util.c ends up using it for
> pagemap_is_swapped() and friends via page_entry_is() to sanity check
> that what pagemap gives us is consistent with what pagemap_scan gives us.
>
> So it should work independent of the uffd magic.
> I might be wrong, though ...


PAGEMAP_SCAN can work without the UFFD magic. CRIU utilizes PAGEMAP_SCAN
as a more efficient alternative to /proc/pid/pagemap:
https://github.com/checkpoint-restore/criu/blob/d18912fc88f3dc7bde5fdfa3575691977eb21753/criu/pagemap-cache.c#L178

For CRIU, obtaining information about guard regions is critical.
Without this functionality in the kernel, CRIU is broken. We probably should
consider backporting these changes to the 6.13 and 6.14 stable branches.

>
> >>
> >>
> >> See do_pagemap_scan().
> >>
> >> The benefit here might be that one could effectively search/filter for guard
> >> regions without copying 64bit per base-page to user space.
> >>
> >> But the idea would be to indicate something like PAGE_IS_GUARD_REGION as a
> >> category when we hit a guard region entry in pagemap_page_category().
> >>
> >> (the code is a bit complicated, and I am not sure why we indicate
> >> PAGE_IS_SWAPPED for non-swap entries, likely wrong ...)
> >
> > Yeah, I could go on here about how much I hate how uffd does a 'parallel
> > implementation' of a ton of stuff and then chucks in if (uffd) { go do
> > something weird + wonderful } but I'll resist the urge :P :))
> >
> > Do you think, if it were uffd-specific, this would be useful?
>
> If it really is completely uffd-specific for now, I agree that we should
> rather leave it alone.
>
> >
> > At any rate, I'm not sure it's _hugely_ beneficial in this form as pagemap
> > is binary in any case so you're not having to deal with overhead of parsing
> > a text file at least!
>
> My thinking was, that if you have a large VMA, with ordinary pagemap you
> have to copy 8byte per entry (and have room for that somewhere in user
> space). In theory, with the scanning feature, you can leave that ...
> scanning to the kernel and don't have to do any copying/allocate space
> for it in user space etc.

PAGEMAP_SCAN doesn't have this issue and it was one of the reasons to
implement it.

Thanks,
Andrei

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-19 18:37 UTC (permalink / raw)
  To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <Z9sCsooW7OSTgyAk@LQ3V64L9R2>

On 3/19/25 11:45 AM, Joe Damato wrote:
> On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
>> On 3/19/25 11:04 AM, Joe Damato wrote:
>>> On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
>>>> On 3/19/25 9:32 AM, Joe Damato wrote:
>>>>> On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
>>>>>> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
>>>>>>> One way to fix this is to add zerocopy notifications to sendfile similar
>>>>>>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
>>>>>>> extensive work done by Pavel [1].
>>>>>>
>>>>>> What is a "zerocopy notification" 
>>>>>
>>>>> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
>>>>> sendmsg and passes MSG_ZEROCOPY a completion notification is added
>>>>> to the error queue. The user app can poll for these to find out when
>>>>> the TX has completed and the buffer it passed to the kernel can be
>>>>> overwritten.
>>>>>
>>>>> My series provides the same functionality via splice and sendfile2.
>>>>>
>>>>> [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
>>>>>
>>>>>> and why aren't you simply plugging this into io_uring and generate
>>>>>> a CQE so that it works like all other asynchronous operations?
>>>>>
>>>>> I linked to the iouring work that Pavel did in the cover letter.
>>>>> Please take a look.
>>>>>
>>>>> That work refactored the internals of how zerocopy completion
>>>>> notifications are wired up, allowing other pieces of code to use the
>>>>> same infrastructure and extend it, if needed.
>>>>>
>>>>> My series is using the same internals that iouring (and others) use
>>>>> to generate zerocopy completion notifications. Unlike iouring,
>>>>> though, I don't need a fully customized implementation with a new
>>>>> user API for harvesting completion events; I can use the existing
>>>>> mechanism already in the kernel that user apps already use for
>>>>> sendmsg (the error queue, as explained above and in the
>>>>> MSG_ZEROCOPY documentation).
>>>>
>>>> The error queue is arguably a work-around for _not_ having a delivery
>>>> mechanism that works with a sync syscall in the first place. The main
>>>> question here imho would be "why add a whole new syscall etc when
>>>> there's already an existing way to do accomplish this, with
>>>> free-to-reuse notifications". If the answer is "because splice", then it
>>>> would seem saner to plumb up those bits only. Would be much simpler
>>>> too...
>>>
>>> I may be misunderstanding your comment, but my response would be:
>>>
>>>   There are existing apps which use sendfile today unsafely and
>>>   it would be very nice to have a safe sendfile equivalent. Converting
>>>   existing apps to using iouring (if I understood your suggestion?)
>>>   would be significantly more work compared to calling sendfile2 and
>>>   adding code to check the error queue.
>>
>> It's really not, if you just want to use it as a sync kind of thing. If
>> you want to have multiple things in flight etc, yeah it could be more
>> work, you'd also get better performance that way. And you could use
>> things like registered buffers for either of them, which again would
>> likely make it more efficient.
> 
> I haven't argued that performance would be better using sendfile2
> compared to iouring, just that existing apps which already use
> sendfile (but do so unsafely) would probably be more likely to use a
> safe alternative with existing examples of how to harvest completion
> notifications vs something more complex, like wrapping iouring.

Sure and I get that, just not sure it'd be worth doing on the kernel
side for such (fairly) weak reasoning. The performance benefit is just a
side note in that if you did do it this way, you'd potentially be able
to run it more efficiently too. And regardless what people do or use
now, they are generally always interested in that aspect.

>> If you just use it as a sync thing, it'd be pretty trivial to just wrap
>> a my_sendfile_foo() in a submit_and_wait operation, which issues and
>> waits on the completion in a single syscall. And if you want to wait on
>> the notification too, you could even do that in the same syscall and
>> wait on 2 CQEs. That'd be a downright trivial way to provide a sync way
>> of doing the same thing.
> 
> I don't disagree; I just don't know if app developers:
>   a.) know that this is possible to do, and
>   b.) know how to do it

Writing that wrapper would be not even a screenful of code. Yes maybe
they don't know how to do it now, but it's _really_ trivial to do. It'd
take me roughly 1 min to do that, would be happy to help out with that
side so it could go into a commit or man page or whatever.

> In general: it does seem a bit odd to me that there isn't a safe
> sendfile syscall in Linux that uses existing completion notification
> mechanisms.

Pretty natural, I think. sendfile(2) predates that by quite a bit, and
the last real change to sendfile was using splice underneath. Which I
did, and that was probably almost 20 years ago at this point...

I do think it makes sense to have a sendfile that's both fast and
efficient, and can be used sanely with buffer reuse without relying on
odd heuristics.

>>> I would also argue that there are likely user apps out there that
>>> use both sendmsg MSG_ZEROCOPY for certain writes (for data in
>>> memory) and also use sendfile (for data on disk). One example would
>>> be a reverse proxy that might write HTTP headers to clients via
>>> sendmsg but transmit the response body with sendfile.
>>>
>>> For those apps, the code to check the error queue already exists for
>>> sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
>>> way to ensure safe sendfile usage.
>>
>> Sure that is certainly possible. I didn't say that wasn't the case,
>> rather that the error queue approach is a work-around in the first place
>> for not having some kind of async notification mechanism for when it's
>> free to reuse.
> 
> Of course, I certainly agree that the error queue is a work around.
> But it works, app use it, and its fairly well known. I don't see any
> reason, other than historical context, why sendmsg can use this
> mechanism, splice can, but sendfile shouldn't?

My argument would be the same as for other features - if you can do it
simpler this other way, why not consider that? The end result would be
the same, you can do fast sendfile() with sane buffer reuse. But the
kernel side would be simpler, which is always a kernel main goal for
those of us that have to maintain it.

Just adding sendfile2() works in the sense that it's an easier drop in
replacement for an app, though the error queue side does mean it needs
to change anyway - it's not just replacing one syscall with another. And
if we want to be lazy, sure that's fine. I just don't think it's the
best way to do it when we literally have a mechanism that's designed for
this and works with reuse already with normal send zc (and receive side
too, in the next kernel).

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH 1/2] fs/proc/task_mmu: add guard region bit to pagemap
From: Lorenzo Stoakes @ 2025-03-19 19:12 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: David Hildenbrand, Andrew Morton, Jonathan Corbet, Shuah Khan,
	Suren Baghdasaryan, Kalesh Singh, Liam R . Howlett,
	Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
	Juan Yescas, linux-mm, linux-doc, linux-kernel, linux-fsdevel,
	linux-kselftest, linux-api, criu, Alexander Mikhalitsyn,
	Pavel Tikhomirov, Mike Rapoport, Greg Kroah-Hartman
In-Reply-To: <CANaxB-yMBSFeYcTr-PaevooSeHUkCN9GWTUkLZUNW2vxKzm0sg@mail.gmail.com>

+cc Greg for stable question

On Wed, Mar 19, 2025 at 11:22:40AM -0700, Andrei Vagin wrote:
> On Mon, Feb 24, 2025 at 2:39 AM David Hildenbrand <david@redhat.com> wrote:
> >
> > On 24.02.25 11:18, Lorenzo Stoakes wrote:

[snip]
> > >>
> > >> Acked-by: David Hildenbrand <david@redhat.com>
> > >
> > > Thanks! :)
> > >>
> > >> Something that might be interesting is also extending the PAGEMAP_SCAN
> > >> ioctl.
> > >
> > > Yeah, funny you should mention that, I did see that, but on reading the man
> > > page it struck me that it requires the region to be uffd afaict? All the
> > > tests seem to establish uffd, and the man page implies it:
> > >
> > >         To start tracking the written state (flag) of a page or range of
> > >         memory, the UFFD_FEATURE_WP_ASYNC must be enabled by UFFDIO_API
> > >         ioctl(2) on userfaultfd and memory range must be registered with
> > >         UFFDIO_REGISTER ioctl(2) in UFFDIO_REGISTER_MODE_WP mode.
> > >
> > > It would be a bit of a weird edge case to add support there. I was excited
> > > when I first saw this ioctl, then disappointed afterwards... but maybe I
> > > got it wrong?
>
> > >
> >
> > I never managed to review that fully, but I thing that
> > UFFD_FEATURE_WP_ASYNC thingy is only required for PM_SCAN_CHECK_WPASYNC
> > and PM_SCAN_WP_MATCHING.
> >
> > See pagemap_scan_test_walk().
> >
> > I do recall that it works on any VMA.
> >
> > Ah yes, tools/testing/selftests/mm/vm_util.c ends up using it for
> > pagemap_is_swapped() and friends via page_entry_is() to sanity check
> > that what pagemap gives us is consistent with what pagemap_scan gives us.
> >
> > So it should work independent of the uffd magic.
> > I might be wrong, though ...
>
>
> PAGEMAP_SCAN can work without the UFFD magic. CRIU utilizes PAGEMAP_SCAN
> as a more efficient alternative to /proc/pid/pagemap:
> https://github.com/checkpoint-restore/criu/blob/d18912fc88f3dc7bde5fdfa3575691977eb21753/criu/pagemap-cache.c#L178
>

Yeah we ascertained that - is on my list, LSF coming up next week means we
aren't great on timing here, but I'll prioritise this. When I'm back.

> For CRIU, obtaining information about guard regions is critical.
> Without this functionality in the kernel, CRIU is broken. We probably should
> consider backporting these changes to the 6.13 and 6.14 stable branches.
>

I'm not sure on precedent for backporting a feature like this - Greg? Am
happy to do it though.

As a stop gap we can backport the pagemap feature if Greg feels this is
appropriate?

[snip]

> > My thinking was, that if you have a large VMA, with ordinary pagemap you
> > have to copy 8byte per entry (and have room for that somewhere in user
> > space). In theory, with the scanning feature, you can leave that ...
> > scanning to the kernel and don't have to do any copying/allocate space
> > for it in user space etc.
>
> PAGEMAP_SCAN doesn't have this issue and it was one of the reasons to
> implement it.

Ack.

>
> Thanks,
> Andrei

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Stefan Metzmacher @ 2025-03-19 19:15 UTC (permalink / raw)
  To: Jens Axboe, Joe Damato, Christoph Hellwig, netdev, linux-kernel,
	asml.silence, linux-fsdevel, edumazet, pabeni, horms, linux-api,
	linux-arch, viro, jack, kuba, shuah, sdf, mingo, arnd, brauner,
	akpm, tglx, jolsa, linux-kselftest
In-Reply-To: <dc3ebb86-f4b2-443a-9b0d-f5470fd773f1@kernel.dk>

Am 19.03.25 um 19:37 schrieb Jens Axboe:
> On 3/19/25 11:45 AM, Joe Damato wrote:
>> On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
>>> On 3/19/25 11:04 AM, Joe Damato wrote:
>>>> On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
>>>>> On 3/19/25 9:32 AM, Joe Damato wrote:
>>>>>> On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
>>>>>>> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
>>>>>>>> One way to fix this is to add zerocopy notifications to sendfile similar
>>>>>>>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
>>>>>>>> extensive work done by Pavel [1].
>>>>>>>
>>>>>>> What is a "zerocopy notification"
>>>>>>
>>>>>> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
>>>>>> sendmsg and passes MSG_ZEROCOPY a completion notification is added
>>>>>> to the error queue. The user app can poll for these to find out when
>>>>>> the TX has completed and the buffer it passed to the kernel can be
>>>>>> overwritten.
>>>>>>
>>>>>> My series provides the same functionality via splice and sendfile2.
>>>>>>
>>>>>> [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
>>>>>>
>>>>>>> and why aren't you simply plugging this into io_uring and generate
>>>>>>> a CQE so that it works like all other asynchronous operations?
>>>>>>
>>>>>> I linked to the iouring work that Pavel did in the cover letter.
>>>>>> Please take a look.
>>>>>>
>>>>>> That work refactored the internals of how zerocopy completion
>>>>>> notifications are wired up, allowing other pieces of code to use the
>>>>>> same infrastructure and extend it, if needed.
>>>>>>
>>>>>> My series is using the same internals that iouring (and others) use
>>>>>> to generate zerocopy completion notifications. Unlike iouring,
>>>>>> though, I don't need a fully customized implementation with a new
>>>>>> user API for harvesting completion events; I can use the existing
>>>>>> mechanism already in the kernel that user apps already use for
>>>>>> sendmsg (the error queue, as explained above and in the
>>>>>> MSG_ZEROCOPY documentation).
>>>>>
>>>>> The error queue is arguably a work-around for _not_ having a delivery
>>>>> mechanism that works with a sync syscall in the first place. The main
>>>>> question here imho would be "why add a whole new syscall etc when
>>>>> there's already an existing way to do accomplish this, with
>>>>> free-to-reuse notifications". If the answer is "because splice", then it
>>>>> would seem saner to plumb up those bits only. Would be much simpler
>>>>> too...
>>>>
>>>> I may be misunderstanding your comment, but my response would be:
>>>>
>>>>    There are existing apps which use sendfile today unsafely and
>>>>    it would be very nice to have a safe sendfile equivalent. Converting
>>>>    existing apps to using iouring (if I understood your suggestion?)
>>>>    would be significantly more work compared to calling sendfile2 and
>>>>    adding code to check the error queue.
>>>
>>> It's really not, if you just want to use it as a sync kind of thing. If
>>> you want to have multiple things in flight etc, yeah it could be more
>>> work, you'd also get better performance that way. And you could use
>>> things like registered buffers for either of them, which again would
>>> likely make it more efficient.
>>
>> I haven't argued that performance would be better using sendfile2
>> compared to iouring, just that existing apps which already use
>> sendfile (but do so unsafely) would probably be more likely to use a
>> safe alternative with existing examples of how to harvest completion
>> notifications vs something more complex, like wrapping iouring.
> 
> Sure and I get that, just not sure it'd be worth doing on the kernel
> side for such (fairly) weak reasoning. The performance benefit is just a
> side note in that if you did do it this way, you'd potentially be able
> to run it more efficiently too. And regardless what people do or use
> now, they are generally always interested in that aspect.
> 
>>> If you just use it as a sync thing, it'd be pretty trivial to just wrap
>>> a my_sendfile_foo() in a submit_and_wait operation, which issues and
>>> waits on the completion in a single syscall. And if you want to wait on
>>> the notification too, you could even do that in the same syscall and
>>> wait on 2 CQEs. That'd be a downright trivial way to provide a sync way
>>> of doing the same thing.
>>
>> I don't disagree; I just don't know if app developers:
>>    a.) know that this is possible to do, and
>>    b.) know how to do it
> 
> Writing that wrapper would be not even a screenful of code. Yes maybe
> they don't know how to do it now, but it's _really_ trivial to do. It'd
> take me roughly 1 min to do that, would be happy to help out with that
> side so it could go into a commit or man page or whatever.
> 
>> In general: it does seem a bit odd to me that there isn't a safe
>> sendfile syscall in Linux that uses existing completion notification
>> mechanisms.
> 
> Pretty natural, I think. sendfile(2) predates that by quite a bit, and
> the last real change to sendfile was using splice underneath. Which I
> did, and that was probably almost 20 years ago at this point...
> 
> I do think it makes sense to have a sendfile that's both fast and
> efficient, and can be used sanely with buffer reuse without relying on
> odd heuristics.
> 
>>>> I would also argue that there are likely user apps out there that
>>>> use both sendmsg MSG_ZEROCOPY for certain writes (for data in
>>>> memory) and also use sendfile (for data on disk). One example would
>>>> be a reverse proxy that might write HTTP headers to clients via
>>>> sendmsg but transmit the response body with sendfile.
>>>>
>>>> For those apps, the code to check the error queue already exists for
>>>> sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
>>>> way to ensure safe sendfile usage.
>>>
>>> Sure that is certainly possible. I didn't say that wasn't the case,
>>> rather that the error queue approach is a work-around in the first place
>>> for not having some kind of async notification mechanism for when it's
>>> free to reuse.
>>
>> Of course, I certainly agree that the error queue is a work around.
>> But it works, app use it, and its fairly well known. I don't see any
>> reason, other than historical context, why sendmsg can use this
>> mechanism, splice can, but sendfile shouldn't?
> 
> My argument would be the same as for other features - if you can do it
> simpler this other way, why not consider that? The end result would be
> the same, you can do fast sendfile() with sane buffer reuse. But the
> kernel side would be simpler, which is always a kernel main goal for
> those of us that have to maintain it.
> 
> Just adding sendfile2() works in the sense that it's an easier drop in
> replacement for an app, though the error queue side does mean it needs
> to change anyway - it's not just replacing one syscall with another. And
> if we want to be lazy, sure that's fine. I just don't think it's the
> best way to do it when we literally have a mechanism that's designed for
> this and works with reuse already with normal send zc (and receive side
> too, in the next kernel).

A few month (or even years) back, Pavel came up with an idea
to implement some kind of splice into a fixed buffer, if that
would be implemented I guess it would help me in Samba too.
My first usage was on the receive side (from the network).

But the other side might also be possible now we have RWF_DONTCACHE.
Instead of dropping the pages from the page cache, it might
be possible move them to fixed buffer instead.
It would mean the pages would be 'stable' when they are
no longer part of the pagecache.
But maybe my assumption for that is too naive...

Anyway that splice into a fixed buffer would great to have,
as the new IORING_OP_RECV_ZC, requires control over the
hardware queues of the nic and only allows a single process
to provide buffers for that receive queue (at least that's how
I understand it). And that's not possible for multiple process
(maybe not belonging to the same high level application and likely
non-root applications). So it would be great have splice into
fixed buffer as alternative to IORING_OP_SPLICE/IORING_OP_TEE,
as it would be more flexible to use in combination with
IORING_OP_SENDMSG_ZC as well as IORING_OP_WRITE[V]_FIXED with RWF_DONTCACHE.

I guess such a splice into fixed buffer linked to IORING_OP_SENDMSG_ZC
would be the way to simulate the sendfile2() in userspace?

Thanks!
metze

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-19 19:16 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <dc3ebb86-f4b2-443a-9b0d-f5470fd773f1@kernel.dk>

On Wed, Mar 19, 2025 at 12:37:29PM -0600, Jens Axboe wrote:
> On 3/19/25 11:45 AM, Joe Damato wrote:
> > On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
> >> On 3/19/25 11:04 AM, Joe Damato wrote:
> >>> On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
> >>>> On 3/19/25 9:32 AM, Joe Damato wrote:
> >>>>> On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
> >>>>>> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
> >>>>>>> One way to fix this is to add zerocopy notifications to sendfile similar
> >>>>>>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
> >>>>>>> extensive work done by Pavel [1].
> >>>>>>
> >>>>>> What is a "zerocopy notification" 
> >>>>>
> >>>>> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> >>>>> sendmsg and passes MSG_ZEROCOPY a completion notification is added
> >>>>> to the error queue. The user app can poll for these to find out when
> >>>>> the TX has completed and the buffer it passed to the kernel can be
> >>>>> overwritten.
> >>>>>
> >>>>> My series provides the same functionality via splice and sendfile2.
> >>>>>
> >>>>> [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
> >>>>>
> >>>>>> and why aren't you simply plugging this into io_uring and generate
> >>>>>> a CQE so that it works like all other asynchronous operations?
> >>>>>
> >>>>> I linked to the iouring work that Pavel did in the cover letter.
> >>>>> Please take a look.
> >>>>>
> >>>>> That work refactored the internals of how zerocopy completion
> >>>>> notifications are wired up, allowing other pieces of code to use the
> >>>>> same infrastructure and extend it, if needed.
> >>>>>
> >>>>> My series is using the same internals that iouring (and others) use
> >>>>> to generate zerocopy completion notifications. Unlike iouring,
> >>>>> though, I don't need a fully customized implementation with a new
> >>>>> user API for harvesting completion events; I can use the existing
> >>>>> mechanism already in the kernel that user apps already use for
> >>>>> sendmsg (the error queue, as explained above and in the
> >>>>> MSG_ZEROCOPY documentation).
> >>>>
> >>>> The error queue is arguably a work-around for _not_ having a delivery
> >>>> mechanism that works with a sync syscall in the first place. The main
> >>>> question here imho would be "why add a whole new syscall etc when
> >>>> there's already an existing way to do accomplish this, with
> >>>> free-to-reuse notifications". If the answer is "because splice", then it
> >>>> would seem saner to plumb up those bits only. Would be much simpler
> >>>> too...
> >>>
> >>> I may be misunderstanding your comment, but my response would be:
> >>>
> >>>   There are existing apps which use sendfile today unsafely and
> >>>   it would be very nice to have a safe sendfile equivalent. Converting
> >>>   existing apps to using iouring (if I understood your suggestion?)
> >>>   would be significantly more work compared to calling sendfile2 and
> >>>   adding code to check the error queue.
> >>
> >> It's really not, if you just want to use it as a sync kind of thing. If
> >> you want to have multiple things in flight etc, yeah it could be more
> >> work, you'd also get better performance that way. And you could use
> >> things like registered buffers for either of them, which again would
> >> likely make it more efficient.
> > 
> > I haven't argued that performance would be better using sendfile2
> > compared to iouring, just that existing apps which already use
> > sendfile (but do so unsafely) would probably be more likely to use a
> > safe alternative with existing examples of how to harvest completion
> > notifications vs something more complex, like wrapping iouring.
> 
> Sure and I get that, just not sure it'd be worth doing on the kernel
> side for such (fairly) weak reasoning. The performance benefit is just a
> side note in that if you did do it this way, you'd potentially be able
> to run it more efficiently too. And regardless what people do or use
> now, they are generally always interested in that aspect.

Fair enough.

> >> If you just use it as a sync thing, it'd be pretty trivial to just wrap
> >> a my_sendfile_foo() in a submit_and_wait operation, which issues and
> >> waits on the completion in a single syscall. And if you want to wait on
> >> the notification too, you could even do that in the same syscall and
> >> wait on 2 CQEs. That'd be a downright trivial way to provide a sync way
> >> of doing the same thing.
> > 
> > I don't disagree; I just don't know if app developers:
> >   a.) know that this is possible to do, and
> >   b.) know how to do it
> 
> Writing that wrapper would be not even a screenful of code. Yes maybe
> they don't know how to do it now, but it's _really_ trivial to do. It'd
> take me roughly 1 min to do that, would be happy to help out with that
> side so it could go into a commit or man page or whatever.

I'd never be opposed to more documentation ;)

> > In general: it does seem a bit odd to me that there isn't a safe
> > sendfile syscall in Linux that uses existing completion notification
> > mechanisms.
> 
> Pretty natural, I think. sendfile(2) predates that by quite a bit, and
> the last real change to sendfile was using splice underneath. Which I
> did, and that was probably almost 20 years ago at this point...
> 
> I do think it makes sense to have a sendfile that's both fast and
> efficient, and can be used sanely with buffer reuse without relying on
> odd heuristics.

Just trying to tie this together in my head -- are you saying that
you think the kernel internals of sendfile could be changed in a
different way or that this a userland problem (and they should use
the io_uring wrapper you suggested above) ?

> >>> I would also argue that there are likely user apps out there that
> >>> use both sendmsg MSG_ZEROCOPY for certain writes (for data in
> >>> memory) and also use sendfile (for data on disk). One example would
> >>> be a reverse proxy that might write HTTP headers to clients via
> >>> sendmsg but transmit the response body with sendfile.
> >>>
> >>> For those apps, the code to check the error queue already exists for
> >>> sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
> >>> way to ensure safe sendfile usage.
> >>
> >> Sure that is certainly possible. I didn't say that wasn't the case,
> >> rather that the error queue approach is a work-around in the first place
> >> for not having some kind of async notification mechanism for when it's
> >> free to reuse.
> > 
> > Of course, I certainly agree that the error queue is a work around.
> > But it works, app use it, and its fairly well known. I don't see any
> > reason, other than historical context, why sendmsg can use this
> > mechanism, splice can, but sendfile shouldn't?
> 
> My argument would be the same as for other features - if you can do it
> simpler this other way, why not consider that? The end result would be
> the same, you can do fast sendfile() with sane buffer reuse. But the
> kernel side would be simpler, which is always a kernel main goal for
> those of us that have to maintain it.
>
> Just adding sendfile2() works in the sense that it's an easier drop in
> replacement for an app, though the error queue side does mean it needs
> to change anyway - it's not just replacing one syscall with another. And
> if we want to be lazy, sure that's fine. I just don't think it's the
> best way to do it when we literally have a mechanism that's designed for
> this and works with reuse already with normal send zc (and receive side
> too, in the next kernel).

It seems like you've answered the question I asked above and that
you are suggesting there might be a better and simpler sendfile2
kernel-side implementation that doesn't rely on splice internals at
all.

Am I following you? If so, I'll drop the sendfile2 stuff from this
series and stick with the splice changes only, if you are (at a high
level) OK with the idea of adding a flag for this to splice.

In the meantime, I'll take a few more reads through the iouring code
to see if I can work out how sendfile2 might be built on top of that
instead of splice in the kernel.

Thank you very much for your time, feedback, and attention,
Joe

^ permalink raw reply

* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-19 23:22 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, netdev, linux-kernel, asml.silence,
	linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
	viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
	jolsa, linux-kselftest
In-Reply-To: <2d68bc91-c22c-4b48-a06d-fa9ec06dfb25@kernel.dk>

On Wed, Mar 19, 2025 at 10:07:27AM -0600, Jens Axboe wrote:
> On 3/19/25 9:32 AM, Joe Damato wrote:
> > On Wed, Mar 19, 2025 at 01:04:48AM -0700, Christoph Hellwig wrote:
> >> On Wed, Mar 19, 2025 at 12:15:11AM +0000, Joe Damato wrote:
> >>> One way to fix this is to add zerocopy notifications to sendfile similar
> >>> to how MSG_ZEROCOPY works with sendmsg. This is possible thanks to the
> >>> extensive work done by Pavel [1].
> >>
> >> What is a "zerocopy notification" 
> > 
> > See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> > sendmsg and passes MSG_ZEROCOPY a completion notification is added
> > to the error queue. The user app can poll for these to find out when
> > the TX has completed and the buffer it passed to the kernel can be
> > overwritten.
> > 
> > My series provides the same functionality via splice and sendfile2.
> > 
> > [1]: https://www.kernel.org/doc/html/v6.13/networking/msg_zerocopy.html
> > 
> >> and why aren't you simply plugging this into io_uring and generate
> >> a CQE so that it works like all other asynchronous operations?
> > 
> > I linked to the iouring work that Pavel did in the cover letter.
> > Please take a look.
> > 
> > That work refactored the internals of how zerocopy completion
> > notifications are wired up, allowing other pieces of code to use the
> > same infrastructure and extend it, if needed.
> > 
> > My series is using the same internals that iouring (and others) use
> > to generate zerocopy completion notifications. Unlike iouring,
> > though, I don't need a fully customized implementation with a new
> > user API for harvesting completion events; I can use the existing
> > mechanism already in the kernel that user apps already use for
> > sendmsg (the error queue, as explained above and in the
> > MSG_ZEROCOPY documentation).
> 
> The error queue is arguably a work-around for _not_ having a delivery
> mechanism that works with a sync syscall in the first place. The main
> question here imho would be "why add a whole new syscall etc when
> there's already an existing way to do accomplish this, with
> free-to-reuse notifications". If the answer is "because splice", then it
> would seem saner to plumb up those bits only. Would be much simpler
> too...

OK, I reworked the patches to drop all the sendfile2 stuff so no new
system call is added. Only a flag for splice, SPLICE_F_ZC.

It feels weird to add this to the splice path but not the path that
sendfile takes through splice.

I understand and agree with you: if we are adding a new system
call, like sendfile2, it should probably be done as you've
described in your other messages.

What about an alternative?

Would you be open to the idea that sendfile could be extended to
generate error queue completions if the network socket has
SO_ZEROCOPY set?

If so, that would solve the original problem without introducing a
new system call and still leaves the door open for a more efficient
sendfile2 based on iouring internals later.

What do you think?

^ permalink raw reply

* Re: [PATCH 1/2] fs/proc/task_mmu: add guard region bit to pagemap
From: Greg Kroah-Hartman @ 2025-03-19 23:57 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Andrei Vagin, David Hildenbrand, Andrew Morton, Jonathan Corbet,
	Shuah Khan, Suren Baghdasaryan, Kalesh Singh, Liam R . Howlett,
	Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
	Juan Yescas, linux-mm, linux-doc, linux-kernel, linux-fsdevel,
	linux-kselftest, linux-api, criu, Alexander Mikhalitsyn,
	Pavel Tikhomirov, Mike Rapoport
In-Reply-To: <10c3e304-1a6d-45ac-a3ad-7c0c8d00e03f@lucifer.local>

On Wed, Mar 19, 2025 at 07:12:45PM +0000, Lorenzo Stoakes wrote:
> +cc Greg for stable question
> 
> On Wed, Mar 19, 2025 at 11:22:40AM -0700, Andrei Vagin wrote:
> > On Mon, Feb 24, 2025 at 2:39 AM David Hildenbrand <david@redhat.com> wrote:
> > >
> > > On 24.02.25 11:18, Lorenzo Stoakes wrote:
> 
> [snip]
> > > >>
> > > >> Acked-by: David Hildenbrand <david@redhat.com>
> > > >
> > > > Thanks! :)
> > > >>
> > > >> Something that might be interesting is also extending the PAGEMAP_SCAN
> > > >> ioctl.
> > > >
> > > > Yeah, funny you should mention that, I did see that, but on reading the man
> > > > page it struck me that it requires the region to be uffd afaict? All the
> > > > tests seem to establish uffd, and the man page implies it:
> > > >
> > > >         To start tracking the written state (flag) of a page or range of
> > > >         memory, the UFFD_FEATURE_WP_ASYNC must be enabled by UFFDIO_API
> > > >         ioctl(2) on userfaultfd and memory range must be registered with
> > > >         UFFDIO_REGISTER ioctl(2) in UFFDIO_REGISTER_MODE_WP mode.
> > > >
> > > > It would be a bit of a weird edge case to add support there. I was excited
> > > > when I first saw this ioctl, then disappointed afterwards... but maybe I
> > > > got it wrong?
> >
> > > >
> > >
> > > I never managed to review that fully, but I thing that
> > > UFFD_FEATURE_WP_ASYNC thingy is only required for PM_SCAN_CHECK_WPASYNC
> > > and PM_SCAN_WP_MATCHING.
> > >
> > > See pagemap_scan_test_walk().
> > >
> > > I do recall that it works on any VMA.
> > >
> > > Ah yes, tools/testing/selftests/mm/vm_util.c ends up using it for
> > > pagemap_is_swapped() and friends via page_entry_is() to sanity check
> > > that what pagemap gives us is consistent with what pagemap_scan gives us.
> > >
> > > So it should work independent of the uffd magic.
> > > I might be wrong, though ...
> >
> >
> > PAGEMAP_SCAN can work without the UFFD magic. CRIU utilizes PAGEMAP_SCAN
> > as a more efficient alternative to /proc/pid/pagemap:
> > https://github.com/checkpoint-restore/criu/blob/d18912fc88f3dc7bde5fdfa3575691977eb21753/criu/pagemap-cache.c#L178
> >
> 
> Yeah we ascertained that - is on my list, LSF coming up next week means we
> aren't great on timing here, but I'll prioritise this. When I'm back.
> 
> > For CRIU, obtaining information about guard regions is critical.
> > Without this functionality in the kernel, CRIU is broken. We probably should
> > consider backporting these changes to the 6.13 and 6.14 stable branches.
> >
> 
> I'm not sure on precedent for backporting a feature like this - Greg? Am
> happy to do it though.

If it's a regression, sure, we can take it for stable.

> As a stop gap we can backport the pagemap feature if Greg feels this is
> appropriate?

Which do the maintainers of the code feel is appropriate?  I'll defer to
them for making that call :)

thanks,

greg k-h

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox