mbox series

[vfs/for-next,0/3] Move splice_to_socket to net/socket.c

Message ID 20250322203558.206411-1-jdamato@fastly.com (mailing list archive)
Headers show
Series Move splice_to_socket to net/socket.c | expand

Message

Joe Damato March 22, 2025, 8:35 p.m. UTC
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

Comments

Jens Axboe March 24, 2025, 10:14 p.m. UTC | #1
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.
Joe Damato March 24, 2025, 10:51 p.m. UTC | #2
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.