linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c
@ 2025-03-22 20:35 Joe Damato
  2025-03-22 20:35 ` [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice Joe Damato
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Joe Damato @ 2025-03-22 20:35 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: netdev, brauner, asml.silence, hch, axboe, edumazet, kuba, pabeni,
	horms, Joe Damato, Alexander Viro, David S. Miller, Jan Kara,
	open list

Greetings:

While reading through the splice and socket code I noticed that some
splice helpers (like sock_splice_read and sock_splice_eof) live in
net/socket.c, but splice_to_socket does not.

I am not sure if there is a reason for this, but it seems like moving
this code provides some advantages:
  - Eliminates the #ifdef CONFIG_NET from fs/splice.c
  - Keeps the socket related splice helpers together in net/socket.c
    where it seems (IMHO) more logical for them to live

This change is essentially cleanup; no functional changes to splice are
introduced.

I based this change on vfs/for-next since 2 of the 3 patches are vfs,
but I am happy to rebase this on another tree if necessary.

Thanks,
Joe

Joe Damato (3):
  pipe: Move pipe wakeup helpers out of splice
  splice: Move splice_to_socket to net/socket.c
  net: splice_to_socket: RCT declaration cleanup

 fs/pipe.c                 |  16 ++++
 fs/splice.c               | 170 ++------------------------------------
 include/linux/pipe_fs_i.h |   4 +
 include/linux/splice.h    |   3 -
 net/socket.c              | 140 +++++++++++++++++++++++++++++++
 5 files changed, 167 insertions(+), 166 deletions(-)


base-commit: 2e72b1e0aac24a12f3bf3eec620efaca7ab7d4de
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice
  2025-03-22 20:35 [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Joe Damato
@ 2025-03-22 20:35 ` Joe Damato
  2025-03-24 22:15   ` Jens Axboe
  2025-03-22 20:35 ` [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c Joe Damato
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Joe Damato @ 2025-03-22 20:35 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: netdev, brauner, asml.silence, hch, axboe, edumazet, kuba, pabeni,
	horms, Joe Damato, Alexander Viro, Jan Kara, open list

Splice code has helpers to wakeup pipe readers and writers. Move these
helpers out of splice, rename them from "wakeup_pipe_*" to
"pipe_wakeup_*" and update call sites in splice.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 fs/pipe.c                 | 16 ++++++++++++++++
 fs/splice.c               | 34 +++++++++-------------------------
 include/linux/pipe_fs_i.h |  4 ++++
 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 12b22c2723b7..1f496896184b 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1070,6 +1070,22 @@ void pipe_wait_writable(struct pipe_inode_info *pipe)
 	pipe_lock(pipe);
 }
 
+void pipe_wakeup_readers(struct pipe_inode_info *pipe)
+{
+	smp_mb();
+	if (waitqueue_active(&pipe->rd_wait))
+		wake_up_interruptible(&pipe->rd_wait);
+	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
+}
+
+void pipe_wakeup_writers(struct pipe_inode_info *pipe)
+{
+	smp_mb();
+	if (waitqueue_active(&pipe->wr_wait))
+		wake_up_interruptible(&pipe->wr_wait);
+	kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
+}
+
 /*
  * This depends on both the wait (here) and the wakeup (wake_up_partner)
  * holding the pipe lock, so "*cnt" is stable and we know a wakeup cannot
diff --git a/fs/splice.c b/fs/splice.c
index 2898fa1e9e63..dcd594a8fc06 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -175,14 +175,6 @@ static const struct pipe_buf_operations user_page_pipe_buf_ops = {
 	.get		= generic_pipe_buf_get,
 };
 
-static void wakeup_pipe_readers(struct pipe_inode_info *pipe)
-{
-	smp_mb();
-	if (waitqueue_active(&pipe->rd_wait))
-		wake_up_interruptible(&pipe->rd_wait);
-	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
-}
-
 /**
  * splice_to_pipe - fill passed data into a pipe
  * @pipe:	pipe to fill
@@ -414,14 +406,6 @@ const struct pipe_buf_operations nosteal_pipe_buf_ops = {
 };
 EXPORT_SYMBOL(nosteal_pipe_buf_ops);
 
-static void wakeup_pipe_writers(struct pipe_inode_info *pipe)
-{
-	smp_mb();
-	if (waitqueue_active(&pipe->wr_wait))
-		wake_up_interruptible(&pipe->wr_wait);
-	kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
-}
-
 /**
  * splice_from_pipe_feed - feed available data from a pipe to a file
  * @pipe:	pipe to splice from
@@ -541,7 +525,7 @@ static int splice_from_pipe_next(struct pipe_inode_info *pipe, struct splice_des
 			return -ERESTARTSYS;
 
 		if (sd->need_wakeup) {
-			wakeup_pipe_writers(pipe);
+			pipe_wakeup_writers(pipe);
 			sd->need_wakeup = false;
 		}
 
@@ -582,7 +566,7 @@ static void splice_from_pipe_begin(struct splice_desc *sd)
 static void splice_from_pipe_end(struct pipe_inode_info *pipe, struct splice_desc *sd)
 {
 	if (sd->need_wakeup)
-		wakeup_pipe_writers(pipe);
+		pipe_wakeup_writers(pipe);
 }
 
 /**
@@ -837,7 +821,7 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 				goto out;
 
 			if (need_wakeup) {
-				wakeup_pipe_writers(pipe);
+				pipe_wakeup_writers(pipe);
 				need_wakeup = false;
 			}
 
@@ -917,7 +901,7 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 out:
 	pipe_unlock(pipe);
 	if (need_wakeup)
-		wakeup_pipe_writers(pipe);
+		pipe_wakeup_writers(pipe);
 	return spliced ?: ret;
 }
 #endif
@@ -1295,7 +1279,7 @@ ssize_t splice_file_to_pipe(struct file *in,
 		ret = do_splice_read(in, offset, opipe, len, flags);
 	pipe_unlock(opipe);
 	if (ret > 0)
-		wakeup_pipe_readers(opipe);
+		pipe_wakeup_readers(opipe);
 	return ret;
 }
 
@@ -1558,7 +1542,7 @@ static ssize_t vmsplice_to_pipe(struct file *file, struct iov_iter *iter,
 		ret = iter_to_pipe(iter, pipe, buf_flag);
 	pipe_unlock(pipe);
 	if (ret > 0) {
-		wakeup_pipe_readers(pipe);
+		pipe_wakeup_readers(pipe);
 		fsnotify_modify(file);
 	}
 	return ret;
@@ -1844,10 +1828,10 @@ static int splice_pipe_to_pipe(struct pipe_inode_info *ipipe,
 	 * If we put data in the output pipe, wakeup any potential readers.
 	 */
 	if (ret > 0)
-		wakeup_pipe_readers(opipe);
+		pipe_wakeup_readers(opipe);
 
 	if (input_wakeup)
-		wakeup_pipe_writers(ipipe);
+		pipe_wakeup_writers(ipipe);
 
 	return ret;
 }
@@ -1935,7 +1919,7 @@ static ssize_t link_pipe(struct pipe_inode_info *ipipe,
 	 * If we put data in the output pipe, wakeup any potential readers.
 	 */
 	if (ret > 0)
-		wakeup_pipe_readers(opipe);
+		pipe_wakeup_readers(opipe);
 
 	return ret;
 }
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 8ff23bf5a819..de850ef085cb 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -267,6 +267,10 @@ void pipe_double_lock(struct pipe_inode_info *, struct pipe_inode_info *);
 void pipe_wait_readable(struct pipe_inode_info *);
 void pipe_wait_writable(struct pipe_inode_info *);
 
+/* Wake up pipe readers/writers */
+void pipe_wakeup_readers(struct pipe_inode_info *pipe);
+void pipe_wakeup_writers(struct pipe_inode_info *pipe);
+
 struct pipe_inode_info *alloc_pipe_info(void);
 void free_pipe_info(struct pipe_inode_info *);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c
  2025-03-22 20:35 [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Joe Damato
  2025-03-22 20:35 ` [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice Joe Damato
@ 2025-03-22 20:35 ` Joe Damato
  2025-03-24 21:15   ` Jakub Kicinski
  2025-03-22 20:35 ` [PATCH vfs/for-next 3/3] net: splice_to_socket: RCT declaration cleanup Joe Damato
  2025-03-24 22:14 ` [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Jens Axboe
  3 siblings, 1 reply; 10+ messages in thread
From: Joe Damato @ 2025-03-22 20:35 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: netdev, brauner, asml.silence, hch, axboe, edumazet, kuba, pabeni,
	horms, Joe Damato, Alexander Viro, Jan Kara, David S. Miller,
	open list

Eliminate the #ifdef CONFIG_NET from fs/splice.c and move the
splice_to_socket helper to net/socket.c, where the other splice socket
helpers live (like sock_splice_read and sock_splice_eof).

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

diff --git a/fs/splice.c b/fs/splice.c
index dcd594a8fc06..40b96387a515 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -766,146 +766,6 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 
 EXPORT_SYMBOL(iter_file_splice_write);
 
-#ifdef CONFIG_NET
-/**
- * splice_to_socket - splice data from a pipe to a socket
- * @pipe:	pipe to splice from
- * @out:	socket to write to
- * @ppos:	position in @out
- * @len:	number of bytes to splice
- * @flags:	splice modifier flags
- *
- * Description:
- *    Will send @len bytes from the pipe to a network socket. No data copying
- *    is involved.
- *
- */
-ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
-			 loff_t *ppos, size_t len, unsigned int flags)
-{
-	struct socket *sock = sock_from_file(out);
-	struct bio_vec bvec[16];
-	struct msghdr msg = {};
-	ssize_t ret = 0;
-	size_t spliced = 0;
-	bool need_wakeup = false;
-
-	pipe_lock(pipe);
-
-	while (len > 0) {
-		unsigned int head, tail, mask, bc = 0;
-		size_t remain = len;
-
-		/*
-		 * Check for signal early to make process killable when there
-		 * are always buffers available
-		 */
-		ret = -ERESTARTSYS;
-		if (signal_pending(current))
-			break;
-
-		while (pipe_empty(pipe->head, pipe->tail)) {
-			ret = 0;
-			if (!pipe->writers)
-				goto out;
-
-			if (spliced)
-				goto out;
-
-			ret = -EAGAIN;
-			if (flags & SPLICE_F_NONBLOCK)
-				goto out;
-
-			ret = -ERESTARTSYS;
-			if (signal_pending(current))
-				goto out;
-
-			if (need_wakeup) {
-				pipe_wakeup_writers(pipe);
-				need_wakeup = false;
-			}
-
-			pipe_wait_readable(pipe);
-		}
-
-		head = pipe->head;
-		tail = pipe->tail;
-		mask = pipe->ring_size - 1;
-
-		while (!pipe_empty(head, tail)) {
-			struct pipe_buffer *buf = &pipe->bufs[tail & mask];
-			size_t seg;
-
-			if (!buf->len) {
-				tail++;
-				continue;
-			}
-
-			seg = min_t(size_t, remain, buf->len);
-
-			ret = pipe_buf_confirm(pipe, buf);
-			if (unlikely(ret)) {
-				if (ret == -ENODATA)
-					ret = 0;
-				break;
-			}
-
-			bvec_set_page(&bvec[bc++], buf->page, seg, buf->offset);
-			remain -= seg;
-			if (remain == 0 || bc >= ARRAY_SIZE(bvec))
-				break;
-			tail++;
-		}
-
-		if (!bc)
-			break;
-
-		msg.msg_flags = MSG_SPLICE_PAGES;
-		if (flags & SPLICE_F_MORE)
-			msg.msg_flags |= MSG_MORE;
-		if (remain && pipe_occupancy(pipe->head, tail) > 0)
-			msg.msg_flags |= MSG_MORE;
-		if (out->f_flags & O_NONBLOCK)
-			msg.msg_flags |= MSG_DONTWAIT;
-
-		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, bvec, bc,
-			      len - remain);
-		ret = sock_sendmsg(sock, &msg);
-		if (ret <= 0)
-			break;
-
-		spliced += ret;
-		len -= ret;
-		tail = pipe->tail;
-		while (ret > 0) {
-			struct pipe_buffer *buf = &pipe->bufs[tail & mask];
-			size_t seg = min_t(size_t, ret, buf->len);
-
-			buf->offset += seg;
-			buf->len -= seg;
-			ret -= seg;
-
-			if (!buf->len) {
-				pipe_buf_release(pipe, buf);
-				tail++;
-			}
-		}
-
-		if (tail != pipe->tail) {
-			pipe->tail = tail;
-			if (pipe->files)
-				need_wakeup = true;
-		}
-	}
-
-out:
-	pipe_unlock(pipe);
-	if (need_wakeup)
-		pipe_wakeup_writers(pipe);
-	return spliced ?: ret;
-}
-#endif
-
 static int warn_unsupported(struct file *file, const char *op)
 {
 	pr_debug_ratelimited(
diff --git a/include/linux/splice.h b/include/linux/splice.h
index 9dec4861d09f..54c47776469d 100644
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -97,9 +97,6 @@ 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(struct pipe_inode_info *pipe, struct file *out,
-			 loff_t *ppos, size_t len, unsigned int flags);
-
 /*
  * for dynamic pipe sizing
  */
diff --git a/net/socket.c b/net/socket.c
index 9a117248f18f..2640b42cf320 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -132,6 +132,8 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
 				struct pipe_inode_info *pipe, size_t len,
 				unsigned int flags);
 static void sock_splice_eof(struct file *file);
+static ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
+				loff_t *ppos, size_t len, unsigned int flags);
 
 #ifdef CONFIG_PROC_FS
 static void sock_show_fdinfo(struct seq_file *m, struct file *f)
@@ -3719,3 +3721,141 @@ u32 kernel_sock_ip_overhead(struct sock *sk)
 	}
 }
 EXPORT_SYMBOL(kernel_sock_ip_overhead);
+
+/**
+ * splice_to_socket - splice data from a pipe to a socket
+ * @pipe:	pipe to splice from
+ * @out:	socket to write to
+ * @ppos:	position in @out
+ * @len:	number of bytes to splice
+ * @flags:	splice modifier flags
+ *
+ * Description:
+ *    Will send @len bytes from the pipe to a network socket. No data copying
+ *    is involved.
+ *
+ */
+static ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
+				loff_t *ppos, size_t len, unsigned int flags)
+{
+	struct socket *sock = sock_from_file(out);
+	struct bio_vec bvec[16];
+	struct msghdr msg = {};
+	ssize_t ret = 0;
+	size_t spliced = 0;
+	bool need_wakeup = false;
+
+	pipe_lock(pipe);
+
+	while (len > 0) {
+		unsigned int head, tail, mask, bc = 0;
+		size_t remain = len;
+
+		/*
+		 * Check for signal early to make process killable when there
+		 * are always buffers available
+		 */
+		ret = -ERESTARTSYS;
+		if (signal_pending(current))
+			break;
+
+		while (pipe_empty(pipe->head, pipe->tail)) {
+			ret = 0;
+			if (!pipe->writers)
+				goto out;
+
+			if (spliced)
+				goto out;
+
+			ret = -EAGAIN;
+			if (flags & SPLICE_F_NONBLOCK)
+				goto out;
+
+			ret = -ERESTARTSYS;
+			if (signal_pending(current))
+				goto out;
+
+			if (need_wakeup) {
+				pipe_wakeup_writers(pipe);
+				need_wakeup = false;
+			}
+
+			pipe_wait_readable(pipe);
+		}
+
+		head = pipe->head;
+		tail = pipe->tail;
+		mask = pipe->ring_size - 1;
+
+		while (!pipe_empty(head, tail)) {
+			struct pipe_buffer *buf = &pipe->bufs[tail & mask];
+			size_t seg;
+
+			if (!buf->len) {
+				tail++;
+				continue;
+			}
+
+			seg = min_t(size_t, remain, buf->len);
+
+			ret = pipe_buf_confirm(pipe, buf);
+			if (unlikely(ret)) {
+				if (ret == -ENODATA)
+					ret = 0;
+				break;
+			}
+
+			bvec_set_page(&bvec[bc++], buf->page, seg, buf->offset);
+			remain -= seg;
+			if (remain == 0 || bc >= ARRAY_SIZE(bvec))
+				break;
+			tail++;
+		}
+
+		if (!bc)
+			break;
+
+		msg.msg_flags = MSG_SPLICE_PAGES;
+		if (flags & SPLICE_F_MORE)
+			msg.msg_flags |= MSG_MORE;
+		if (remain && pipe_occupancy(pipe->head, tail) > 0)
+			msg.msg_flags |= MSG_MORE;
+		if (out->f_flags & O_NONBLOCK)
+			msg.msg_flags |= MSG_DONTWAIT;
+
+		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, bvec, bc,
+			      len - remain);
+		ret = sock_sendmsg(sock, &msg);
+		if (ret <= 0)
+			break;
+
+		spliced += ret;
+		len -= ret;
+		tail = pipe->tail;
+		while (ret > 0) {
+			struct pipe_buffer *buf = &pipe->bufs[tail & mask];
+			size_t seg = min_t(size_t, ret, buf->len);
+
+			buf->offset += seg;
+			buf->len -= seg;
+			ret -= seg;
+
+			if (!buf->len) {
+				pipe_buf_release(pipe, buf);
+				tail++;
+			}
+		}
+
+		if (tail != pipe->tail) {
+			pipe->tail = tail;
+			if (pipe->files)
+				need_wakeup = true;
+		}
+	}
+
+out:
+	pipe_unlock(pipe);
+	if (need_wakeup)
+		pipe_wakeup_writers(pipe);
+	return spliced ?: ret;
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH vfs/for-next 3/3] net: splice_to_socket: RCT declaration cleanup
  2025-03-22 20:35 [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Joe Damato
  2025-03-22 20:35 ` [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice Joe Damato
  2025-03-22 20:35 ` [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c Joe Damato
@ 2025-03-22 20:35 ` Joe Damato
  2025-03-24 22:14 ` [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Jens Axboe
  3 siblings, 0 replies; 10+ messages in thread
From: Joe Damato @ 2025-03-22 20:35 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: netdev, brauner, asml.silence, hch, axboe, edumazet, kuba, pabeni,
	horms, Joe Damato, David S. Miller, open list

Make declarations reverse x-mas tree style now that splice_to_socket
lives in net/.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 net/socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 2640b42cf320..b54df75af1a1 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3739,11 +3739,11 @@ static ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 				loff_t *ppos, size_t len, unsigned int flags)
 {
 	struct socket *sock = sock_from_file(out);
+	bool need_wakeup = false;
 	struct bio_vec bvec[16];
 	struct msghdr msg = {};
-	ssize_t ret = 0;
 	size_t spliced = 0;
-	bool need_wakeup = false;
+	ssize_t ret = 0;
 
 	pipe_lock(pipe);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c
  2025-03-22 20:35 ` [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c Joe Damato
@ 2025-03-24 21:15   ` Jakub Kicinski
  2025-03-24 22:53     ` Joe Damato
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2025-03-24 21:15 UTC (permalink / raw)
  To: Joe Damato
  Cc: linux-fsdevel, netdev, brauner, asml.silence, hch, axboe,
	edumazet, pabeni, horms, Alexander Viro, Jan Kara,
	David S. Miller, open list

On Sat, 22 Mar 2025 20:35:45 +0000 Joe Damato wrote:
> Eliminate the #ifdef CONFIG_NET from fs/splice.c and move the
> splice_to_socket helper to net/socket.c, where the other splice socket
> helpers live (like sock_splice_read and sock_splice_eof).
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>

Matter of preference, to some extent, but FWIW:

Acked-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c
  2025-03-22 20:35 [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Joe Damato
                   ` (2 preceding siblings ...)
  2025-03-22 20:35 ` [PATCH vfs/for-next 3/3] net: splice_to_socket: RCT declaration cleanup Joe Damato
@ 2025-03-24 22:14 ` Jens Axboe
  2025-03-24 22:51   ` Joe Damato
  3 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2025-03-24 22:14 UTC (permalink / raw)
  To: Joe Damato, linux-fsdevel
  Cc: netdev, brauner, asml.silence, hch, edumazet, kuba, pabeni, horms,
	Alexander Viro, David S. Miller, Jan Kara, open list

On 3/22/25 2:35 PM, Joe Damato wrote:
> Greetings:
> 
> While reading through the splice and socket code I noticed that some
> splice helpers (like sock_splice_read and sock_splice_eof) live in
> net/socket.c, but splice_to_socket does not.
> 
> I am not sure if there is a reason for this, but it seems like moving
> this code provides some advantages:
>   - Eliminates the #ifdef CONFIG_NET from fs/splice.c
>   - Keeps the socket related splice helpers together in net/socket.c
>     where it seems (IMHO) more logical for them to live

Not sure I think this is a good idea. Always nice to get rid of some
ifdefs, but the code really should be where it's mostly related to, and
the socket splice helpers have very little to do with the networking
code, it's mostly just pure splice code.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice
  2025-03-22 20:35 ` [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice Joe Damato
@ 2025-03-24 22:15   ` Jens Axboe
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2025-03-24 22:15 UTC (permalink / raw)
  To: Joe Damato, linux-fsdevel
  Cc: netdev, brauner, asml.silence, hch, edumazet, kuba, pabeni, horms,
	Alexander Viro, Jan Kara, open list

On 3/22/25 2:35 PM, Joe Damato wrote:
> Splice code has helpers to wakeup pipe readers and writers. Move these
> helpers out of splice, rename them from "wakeup_pipe_*" to
> "pipe_wakeup_*" and update call sites in splice.

This looks good to me, as it's moving the code to where it belongs.
One minor note:

> +void pipe_wakeup_readers(struct pipe_inode_info *pipe)
> +{
> +	smp_mb();
> +	if (waitqueue_active(&pipe->rd_wait))
> +		wake_up_interruptible(&pipe->rd_wait);
> +	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
> +}
> +
> +void pipe_wakeup_writers(struct pipe_inode_info *pipe)
> +{
> +	smp_mb();
> +	if (waitqueue_active(&pipe->wr_wait))
> +		wake_up_interruptible(&pipe->wr_wait);
> +	kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
> +}

Both of these really should use wq_has_sleeper() - not related to your
change, as it makes more sense to keep the code while moving it. But
just spotted it while looking at it, just a note for the future... In
any case:

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c
  2025-03-24 22:14 ` [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Jens Axboe
@ 2025-03-24 22:51   ` Joe Damato
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Damato @ 2025-03-24 22:51 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-fsdevel, netdev, brauner, asml.silence, hch, edumazet, kuba,
	pabeni, horms, Alexander Viro, David S. Miller, Jan Kara,
	open list

On Mon, Mar 24, 2025 at 04:14:06PM -0600, Jens Axboe wrote:
> On 3/22/25 2:35 PM, Joe Damato wrote:
> > Greetings:
> > 
> > While reading through the splice and socket code I noticed that some
> > splice helpers (like sock_splice_read and sock_splice_eof) live in
> > net/socket.c, but splice_to_socket does not.
> > 
> > I am not sure if there is a reason for this, but it seems like moving
> > this code provides some advantages:
> >   - Eliminates the #ifdef CONFIG_NET from fs/splice.c
> >   - Keeps the socket related splice helpers together in net/socket.c
> >     where it seems (IMHO) more logical for them to live
> 
> Not sure I think this is a good idea. Always nice to get rid of some
> ifdefs, but the code really should be where it's mostly related to, and
> the socket splice helpers have very little to do with the networking
> code, it's mostly just pure splice code.

OK, if you prefer not to merge this I totally understand.

I am not aware of the history behind it all and I can definitely see
the argument for leaving it as is because the code might be more
"splice-related" than networking.

In which case: sorry for the noise.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c
  2025-03-24 21:15   ` Jakub Kicinski
@ 2025-03-24 22:53     ` Joe Damato
  2025-03-25 14:40       ` Christian Brauner
  0 siblings, 1 reply; 10+ messages in thread
From: Joe Damato @ 2025-03-24 22:53 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: linux-fsdevel, netdev, brauner, asml.silence, hch, axboe,
	edumazet, pabeni, horms, Alexander Viro, Jan Kara,
	David S. Miller, open list

On Mon, Mar 24, 2025 at 02:15:26PM -0700, Jakub Kicinski wrote:
> On Sat, 22 Mar 2025 20:35:45 +0000 Joe Damato wrote:
> > Eliminate the #ifdef CONFIG_NET from fs/splice.c and move the
> > splice_to_socket helper to net/socket.c, where the other splice socket
> > helpers live (like sock_splice_read and sock_splice_eof).
> > 
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> 
> Matter of preference, to some extent, but FWIW:
> 
> Acked-by: Jakub Kicinski <kuba@kernel.org>

Thanks for the ACK.

It looks like Jens thinks maybe the code should stay where it is and
given that it might be more "splice related" than networking, it may
be better after all to leave it where it is.

In which case, my apologies for the noise.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c
  2025-03-24 22:53     ` Joe Damato
@ 2025-03-25 14:40       ` Christian Brauner
  0 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2025-03-25 14:40 UTC (permalink / raw)
  To: Joe Damato
  Cc: Jakub Kicinski, linux-fsdevel, netdev, asml.silence, hch, axboe,
	edumazet, pabeni, horms, Alexander Viro, Jan Kara,
	David S. Miller, open list

On Mon, Mar 24, 2025 at 03:53:23PM -0700, Joe Damato wrote:
> On Mon, Mar 24, 2025 at 02:15:26PM -0700, Jakub Kicinski wrote:
> > On Sat, 22 Mar 2025 20:35:45 +0000 Joe Damato wrote:
> > > Eliminate the #ifdef CONFIG_NET from fs/splice.c and move the
> > > splice_to_socket helper to net/socket.c, where the other splice socket
> > > helpers live (like sock_splice_read and sock_splice_eof).
> > > 
> > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > 
> > Matter of preference, to some extent, but FWIW:
> > 
> > Acked-by: Jakub Kicinski <kuba@kernel.org>
> 
> Thanks for the ACK.
> 
> It looks like Jens thinks maybe the code should stay where it is and
> given that it might be more "splice related" than networking, it may

Uhm, it should stay in fs/ especially since it's closely tied to
pipe_lock().

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-03-25 14:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-22 20:35 [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Joe Damato
2025-03-22 20:35 ` [PATCH vfs/for-next 1/3] pipe: Move pipe wakeup helpers out of splice Joe Damato
2025-03-24 22:15   ` Jens Axboe
2025-03-22 20:35 ` [PATCH vfs/for-next 2/3] splice: Move splice_to_socket to net/socket.c Joe Damato
2025-03-24 21:15   ` Jakub Kicinski
2025-03-24 22:53     ` Joe Damato
2025-03-25 14:40       ` Christian Brauner
2025-03-22 20:35 ` [PATCH vfs/for-next 3/3] net: splice_to_socket: RCT declaration cleanup Joe Damato
2025-03-24 22:14 ` [PATCH vfs/for-next 0/3] Move splice_to_socket to net/socket.c Jens Axboe
2025-03-24 22:51   ` Joe Damato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).